[PATCH] Including device.h and resource.h header files in linux/amba/bus.h

[PATCH] Including device.h and resource.h header files in linux/amba/bus.h

[Viresh KUMAR]

linux/amba/bus.h have dependencies on linux/device.h and linux/resource.h, but
it doesn't include them. We get compilation errors in our files which include
bus.h but doesn't include device.h and resource.h. This patch includes device.h
and resource.h in linux/amba/bus.h file.

Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
---
 include/linux/amba/bus.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/include/linux/amba/bus.h b/include/linux/amba/bus.h
index 6816be6..8b10386 100644
--- a/include/linux/amba/bus.h
+++ b/include/linux/amba/bus.h
@@ -14,6 +14,9 @@
 #ifndef ASMARM_AMBA_H
 #define ASMARM_AMBA_H
 
+#include <linux/device.h>
+#include <linux/resource.h>
+
 #define AMBA_NR_IRQS	2
 
 struct amba_device {
-- 
1.6.0.2

[PATCH] removing compilation warning comming from clkdev.h

[Viresh KUMAR]

clkdev.h is using struct device *. Due to this compilation
warning is comming. Removing this warning.

Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
---
 arch/arm/include/asm/clkdev.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/clkdev.h b/arch/arm/include/asm/clkdev.h
index 7a0690d..b56c138 100644
--- a/arch/arm/include/asm/clkdev.h
+++ b/arch/arm/include/asm/clkdev.h
@@ -13,6 +13,7 @@
 #define __ASM_CLKDEV_H
 
 struct clk;
+struct device;
 
 struct clk_lookup {
 	struct list_head	node;
-- 
1.6.0.2

[PATCH] Including device.h and resource.h header files in linux/amba/bus.h

[Linus Walleij]

2010/3/25 Viresh KUMAR <viresh.kumar@st.com>:

> linux/amba/bus.h have dependencies on linux/device.h and linux/resource.h, but
> it doesn't include them. We get compilation errors in our files which include
> bus.h but doesn't include device.h and resource.h. This patch includes device.h
> and resource.h in linux/amba/bus.h file.

The customs of the kernel is to not #include .h files into each other so much,
the reason being that this easily goes out of hand. Instead #include
device.h and resource.h into your sourcefile above the amba/bus.h
inclusion.

Yours,
Linus Walleij

[PATCH] Including device.h and resource.h header files in linux/amba/bus.h

[viresh kumar]

On Fri, Mar 26, 2010 at 11:01 PM, Linus Walleij
<linus.ml.walleij@gmail.com> wrote:
> 2010/3/25 Viresh KUMAR <viresh.kumar@st.com>:
>
>> linux/amba/bus.h have dependencies on linux/device.h and linux/resource.h, but
>> it doesn't include them. We get compilation errors in our files which include
>> bus.h but doesn't include device.h and resource.h. This patch includes device.h
>> and resource.h in linux/amba/bus.h file.
>
> The customs of the kernel is to not #include .h files into each other so much,
> the reason being that this easily goes out of hand. Instead #include
> device.h and resource.h into your sourcefile above the amba/bus.h
> inclusion.
>

Some time back i have initiated a thread regarding this type of issues. You can
find them here:

http://lists.infradead.org/pipermail/linux-arm-kernel/2010-February/010258.html

And my understanding after the discussion was:

It is the responsibility of a header files, using types definitions
from outside world,
to include all header files required for proper compilation.
This is not the responsibility of the user using this file to add
dependency header in
his source files.

Ex: As everybody who is going to use bus.h has to have device.h and
resource.h included.
So why not have all of them included once only. So that people don't
see compilation
warnings when they try to use these header files. And don't have to
add them in all source
files using them.

What do you say?

regards,
viresh kumar.

[PATCH] Including device.h and resource.h header files in linux/amba/bus.h

[Linus Walleij]

2010/3/27 viresh kumar <viresh.linux@gmail.com>:

> Some time back i have initiated a thread regarding this type of issues. You can
> find them here:
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2010-February/010258.html
>
> And my understanding after the discussion was:
>
> It is the responsibility of a header files, using types definitions
> from outside world,
> to include all header files required for proper compilation.
> This is not the responsibility of the user using this file to add
> dependency header in
> his source files.

On the other hand Documentation/SubmitChecklist contains this:

1: If you use a facility then #include the file that defines/declares
   that facility.  Don't depend on other header files pulling in ones
   that you use.

Then this falls back on the definition of "facility". (And other academic
discussions.)

I guess Russell will need to decide on this.

Linus

[PATCH] Including device.h and resource.h header files in linux/amba/bus.h

[viresh kumar]

On Sat, Mar 27, 2010 at 2:43 PM, Linus Walleij
<linus.ml.walleij@gmail.com> wrote:
> 2010/3/27 viresh kumar <viresh.linux@gmail.com>:
>
>> Some time back i have initiated a thread regarding this type of issues. You can
>> find them here:
>>
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2010-February/010258.html
>>
>> And my understanding after the discussion was:
>>
>> It is the responsibility of a header files, using types definitions
>> from outside world,
>> to include all header files required for proper compilation.
>> This is not the responsibility of the user using this file to add
>> dependency header in
>> his source files.
>
> On the other hand Documentation/SubmitChecklist contains this:
>
> 1: If you use a facility then #include the file that defines/declares
> ? that facility. ?Don't depend on other header files pulling in ones
> ? that you use.

I agree on the above statement.

But what if i don't want to use every type that is used in header file
i am including. For ex: I don't want to use resource
structure in my code and i need to include bus.h for some
other definition. Then why should i get a compilation warning.

Again, according to above statement from submitchecklist:
shouldn't bus.h include device.h and resource.h, as it is using
these facilities.

regards,
viresh kumar.

[PATCH] Including device.h and resource.h header files in linux/amba/bus.h

[Linus Walleij]

[Viresh]
> On Sat, Mar 27, 2010 at 2:43 PM, Linus Walleij
>> On the other hand Documentation/SubmitChecklist contains this:
>>
>> 1: If you use a facility then #include the file that defines/declares
>> ? that facility. ?Don't depend on other header files pulling in ones
>> ? that you use.
>
> I agree on the above statement.
>
>(...)
> Again, according to above statement from submitchecklist:
> shouldn't bus.h include device.h and resource.h, as it is using
> these facilities.

Yes I think you're right, looks like it should be applied then!
Acked-by:

Yours,
Linus Walleij

[PATCH] Including device.h and resource.h header files in linux/amba/bus.h

[Russell King - ARM Linux]

On Thu, Mar 25, 2010 at 03:33:32PM +0530, Viresh KUMAR wrote:
> linux/amba/bus.h have dependencies on linux/device.h and linux/resource.h, but
> it doesn't include them. We get compilation errors in our files which include
> bus.h but doesn't include device.h and resource.h. This patch includes device.h
> and resource.h in linux/amba/bus.h file.

Ok.

[PATCH] removing compilation warning comming from clkdev.h

[Russell King - ARM Linux]

On Thu, Mar 25, 2010 at 03:33:33PM +0530, Viresh KUMAR wrote:
> clkdev.h is using struct device *. Due to this compilation
> warning is comming. Removing this warning.

Ok.

[PATCH] Including device.h and resource.h header files in linux/amba/bus.h

[H Hartley Sweeten]

On Saturday, March 27, 2010 1:57 PM, Linus Walleij wrote:
>> On Sat, Mar 27, 2010 at 2:43 PM, Linus Walleij
>>> On the other hand Documentation/SubmitChecklist contains this:
>>>
>>> 1: If you use a facility then #include the file that defines/declares
>>> ? that facility. ?Don't depend on other header files pulling in ones
>>> ? that you use.
>>
>> I agree on the above statement.
>>
>>(...)
>> Again, according to above statement from submitchecklist:
>> shouldn't bus.h include device.h and resource.h, as it is using
>> these facilities.
>
> Yes I think you're right, looks like it should be applied then!
> Acked-by:

A cleaner solution would be to just add:

struct device;
struct resource;

to the bus.h header.  This will pacify the precompiler.  Then if the
source file actually does need the device of resource facilities you
will get a proper error message during the compile to indicate this.
Adding the #includes to this header means many source files already
in the tree are going to needlessly include the headers twice.

The only reason the include the headers directly in bus.h is if something
(i.e. an inline function) uses the facilities provided by the included
header.  Nothing in bus.h does.

Regards,
Hartley

[PATCH] Including device.h and resource.h header files in linux/amba/bus.h

[Russell King - ARM Linux]

On Mon, Mar 29, 2010 at 12:16:32PM -0500, H Hartley Sweeten wrote:
> On Saturday, March 27, 2010 1:57 PM, Linus Walleij wrote:
> >> On Sat, Mar 27, 2010 at 2:43 PM, Linus Walleij
> >>> On the other hand Documentation/SubmitChecklist contains this:
> >>>
> >>> 1: If you use a facility then #include the file that defines/declares
> >>> ? that facility. ?Don't depend on other header files pulling in ones
> >>> ? that you use.
> >>
> >> I agree on the above statement.
> >>
> >>(...)
> >> Again, according to above statement from submitchecklist:
> >> shouldn't bus.h include device.h and resource.h, as it is using
> >> these facilities.
> >
> > Yes I think you're right, looks like it should be applied then!
> > Acked-by:
> 
> A cleaner solution would be to just add:
> 
> struct device;
> struct resource;

No - it needs the definitions themselves because it otherwise doesn't
know how to lay out struct amba_device.

What you suggest works fine for pointers, but not if the actual definition
itself is required.

[PATCH] Including device.h and resource.h header files in linux/amba/bus.h

[H Hartley Sweeten]

On Monday, March 29, 2010 10:20 AM, Russell King wrote:
> On Mon, Mar 29, 2010 at 12:16:32PM -0500, H Hartley Sweeten wrote:
>> On Saturday, March 27, 2010 1:57 PM, Linus Walleij wrote:
>>>> On Sat, Mar 27, 2010 at 2:43 PM, Linus Walleij
>>>>> On the other hand Documentation/SubmitChecklist contains this:
>>>>>
>>>>> 1: If you use a facility then #include the file that defines/declares
>>>>> ? that facility. ?Don't depend on other header files pulling in ones
>>>>> ? that you use.
>>>>
>>>> I agree on the above statement.
>>>>
>>>>(...)
>>>> Again, according to above statement from submitchecklist:
>>>> shouldn't bus.h include device.h and resource.h, as it is using
>>>> these facilities.
>>>
>>> Yes I think you're right, looks like it should be applied then!
>>> Acked-by:
>> 
>> A cleaner solution would be to just add:
>> 
>> struct device;
>> struct resource;
>
> No - it needs the definitions themselves because it otherwise doesn't
> know how to lay out struct amba_device.
>
> What you suggest works fine for pointers, but not if the actual definition
> itself is required.

Oops my bad... I didn't notice that the variables were not pointers.

You are correct, the headers are needed to get the proper definitions.

Regards,
Hartley