[PATCH] cpuidle: add missing header include

[PATCH] cpuidle: add missing header include

[Jingoo Han]

This patch adds missing device.h header to fix build warnings as below:

drivers/cpuidle/cpuidle.h:26:41: warning: 'struct device' declared inside parameter list [enabled by default]
drivers/cpuidle/cpuidle.h:26:41: warning: its scope is only this definition or declaration, which is probably not what you want
[enabled by default]
drivers/cpuidle/cpuidle.h:27:45: warning: 'struct device' declared inside parameter list [enabled by default]
In file included from drivers/cpuidle/driver.c:15:0:
drivers/cpuidle/cpuidle.h:26:41: warning: 'struct device' declared inside parameter list [enabled by default]
drivers/cpuidle/cpuidle.h:26:41: warning: its scope is only this definition or declaration, which is probably not what you want
[enabled by default]
drivers/cpuidle/cpuidle.h:27:45: warning: 'struct device' declared inside parameter list [enabled by default]

This build warning is introduced by commit efeca1b
"cpuidle / sysfs: change function parameter".

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/cpuidle/cpuidle.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/cpuidle/cpuidle.h b/drivers/cpuidle/cpuidle.h
index a5bbd1c..2120d9e 100644
--- a/drivers/cpuidle/cpuidle.h
+++ b/drivers/cpuidle/cpuidle.h
@@ -5,6 +5,8 @@
 #ifndef __DRIVER_CPUIDLE_H
 #define __DRIVER_CPUIDLE_H
 
+#include <linux/device.h>
+
 /* For internal use only */
 extern struct cpuidle_governor *cpuidle_curr_governor;
 extern struct list_head cpuidle_governors;
-- 
1.7.1

Re: [PATCH] cpuidle: add missing header include

[Rafael J. Wysocki]

On Friday, October 26, 2012 01:30:07 PM Jingoo Han wrote:
> This patch adds missing device.h header to fix build warnings as below:
> 
> drivers/cpuidle/cpuidle.h:26:41: warning: 'struct device' declared inside parameter list [enabled by default]
> drivers/cpuidle/cpuidle.h:26:41: warning: its scope is only this definition or declaration, which is probably not what you want
> [enabled by default]
> drivers/cpuidle/cpuidle.h:27:45: warning: 'struct device' declared inside parameter list [enabled by default]
> In file included from drivers/cpuidle/driver.c:15:0:
> drivers/cpuidle/cpuidle.h:26:41: warning: 'struct device' declared inside parameter list [enabled by default]
> drivers/cpuidle/cpuidle.h:26:41: warning: its scope is only this definition or declaration, which is probably not what you want
> [enabled by default]
> drivers/cpuidle/cpuidle.h:27:45: warning: 'struct device' declared inside parameter list [enabled by default]
> 
> This build warning is introduced by commit efeca1b
> "cpuidle / sysfs: change function parameter".
> 
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>

I fixed up the original patch.

Daniel, I must say I'm less and less impressed by the quality of code
I'm getting from you.  Please improve it.

Thanks,
Rafael


> ---
>  drivers/cpuidle/cpuidle.h |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/cpuidle/cpuidle.h b/drivers/cpuidle/cpuidle.h
> index a5bbd1c..2120d9e 100644
> --- a/drivers/cpuidle/cpuidle.h
> +++ b/drivers/cpuidle/cpuidle.h
> @@ -5,6 +5,8 @@
>  #ifndef __DRIVER_CPUIDLE_H
>  #define __DRIVER_CPUIDLE_H
>  
> +#include <linux/device.h>
> +
>  /* For internal use only */
>  extern struct cpuidle_governor *cpuidle_curr_governor;
>  extern struct list_head cpuidle_governors;
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH] cpuidle: add missing header include

[Daniel Lezcano]

On 10/26/2012 01:14 PM, Rafael J. Wysocki wrote:
> On Friday, October 26, 2012 01:30:07 PM Jingoo Han wrote:
>> This patch adds missing device.h header to fix build warnings as below:
>>
>> drivers/cpuidle/cpuidle.h:26:41: warning: 'struct device' declared inside parameter list [enabled by default]
>> drivers/cpuidle/cpuidle.h:26:41: warning: its scope is only this definition or declaration, which is probably not what you want
>> [enabled by default]
>> drivers/cpuidle/cpuidle.h:27:45: warning: 'struct device' declared inside parameter list [enabled by default]
>> In file included from drivers/cpuidle/driver.c:15:0:
>> drivers/cpuidle/cpuidle.h:26:41: warning: 'struct device' declared inside parameter list [enabled by default]
>> drivers/cpuidle/cpuidle.h:26:41: warning: its scope is only this definition or declaration, which is probably not what you want
>> [enabled by default]
>> drivers/cpuidle/cpuidle.h:27:45: warning: 'struct device' declared inside parameter list [enabled by default]
>>
>> This build warning is introduced by commit efeca1b
>> "cpuidle / sysfs: change function parameter".
>>
>> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> 
> I fixed up the original patch.
> 
> Daniel, I must say I'm less and less impressed by the quality of code
> I'm getting from you.  Please improve it.

Rafael,

I am sorry there are some compilation problems with the code.

I am trying to improve and cleanup step by step the code. I would
understand you remark if I send you untested patches but I always take
care of making the patches git-bisect safe, compile and boot for every
patch.

I double checked the patch and I was able to compile without problem
with my config file.

Jingoo, would you mind to pastebin your config file please ?

If I can compile without problem and Jingoo can't compile, that means
there is something wrong with the headers and including device.h in the
cpuidle.h does not solve the root problem. Furthermore, driver.c does
not use the struct device.

At the first glance, adding a forward declaration in cpuidle.h

  struct device;

and including device.h for sysfs.c should fix the problem.

There are so many combinations for the configuration that it makes
possible I miss one config option and when the code defines implicit
rules difficult to track like the _COMPONENT macro in acpi, that makes
it harder.

For example, include/linux/pm_qos.h includes device.h, but it is not
needed because a forward declaration should suffice and it is up to the
.c files to include device.h if they manipulate the struct device.
If I fix that, it is possible, with my best effort, to miss a .c to add
the device.h file if the option is not set for it.

I promise to try improving the code but please accept some parts of the
current code look like a jackstraws, don't blame me too much if
sometimes I introduce some errors.

Thanks
  -- Daniel


>> ---
>>  drivers/cpuidle/cpuidle.h |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/cpuidle/cpuidle.h b/drivers/cpuidle/cpuidle.h
>> index a5bbd1c..2120d9e 100644
>> --- a/drivers/cpuidle/cpuidle.h
>> +++ b/drivers/cpuidle/cpuidle.h
>> @@ -5,6 +5,8 @@
>>  #ifndef __DRIVER_CPUIDLE_H
>>  #define __DRIVER_CPUIDLE_H
>>  
>> +#include <linux/device.h>
>> +
>>  /* For internal use only */
>>  extern struct cpuidle_governor *cpuidle_curr_governor;
>>  extern struct list_head cpuidle_governors;
>>


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH] cpuidle: add missing header include

[Daniel Lezcano]

On 10/26/2012 06:30 AM, Jingoo Han wrote:
> This patch adds missing device.h header to fix build warnings as below:
> 
> drivers/cpuidle/cpuidle.h:26:41: warning: 'struct device' declared inside parameter list [enabled by default]
> drivers/cpuidle/cpuidle.h:26:41: warning: its scope is only this definition or declaration, which is probably not what you want
> [enabled by default]
> drivers/cpuidle/cpuidle.h:27:45: warning: 'struct device' declared inside parameter list [enabled by default]
> In file included from drivers/cpuidle/driver.c:15:0:
> drivers/cpuidle/cpuidle.h:26:41: warning: 'struct device' declared inside parameter list [enabled by default]
> drivers/cpuidle/cpuidle.h:26:41: warning: its scope is only this definition or declaration, which is probably not what you want
> [enabled by default]
> drivers/cpuidle/cpuidle.h:27:45: warning: 'struct device' declared inside parameter list [enabled by default]
> 
> This build warning is introduced by commit efeca1b
> "cpuidle / sysfs: change function parameter".
> 
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---

Jingoo, could you copy-pastebin your config file. I don't have this
warning and I would like to understand why.

Thanks
  -- Daniel

>  drivers/cpuidle/cpuidle.h |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/cpuidle/cpuidle.h b/drivers/cpuidle/cpuidle.h
> index a5bbd1c..2120d9e 100644
> --- a/drivers/cpuidle/cpuidle.h
> +++ b/drivers/cpuidle/cpuidle.h
> @@ -5,6 +5,8 @@
>  #ifndef __DRIVER_CPUIDLE_H
>  #define __DRIVER_CPUIDLE_H
>  
> +#include <linux/device.h>
> +
>  /* For internal use only */
>  extern struct cpuidle_governor *cpuidle_curr_governor;
>  extern struct list_head cpuidle_governors;


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH] cpuidle: add missing header include

[Jingoo Han]

On Monday, October 29, 2012 6:49 AM Marek Vasut wrote
> 
> On 10/26/2012 06:30 AM, Jingoo Han wrote:
> > This patch adds missing device.h header to fix build warnings as below:
> >
> > drivers/cpuidle/cpuidle.h:26:41: warning: 'struct device' declared inside parameter list [enabled by
> default]
> > drivers/cpuidle/cpuidle.h:26:41: warning: its scope is only this definition or declaration, which is
> probably not what you want
> > [enabled by default]
> > drivers/cpuidle/cpuidle.h:27:45: warning: 'struct device' declared inside parameter list [enabled by
> default]
> > In file included from drivers/cpuidle/driver.c:15:0:
> > drivers/cpuidle/cpuidle.h:26:41: warning: 'struct device' declared inside parameter list [enabled by
> default]
> > drivers/cpuidle/cpuidle.h:26:41: warning: its scope is only this definition or declaration, which is
> probably not what you want
> > [enabled by default]
> > drivers/cpuidle/cpuidle.h:27:45: warning: 'struct device' declared inside parameter list [enabled by
> default]
> >
> > This build warning is introduced by commit efeca1b
> > "cpuidle / sysfs: change function parameter".
> >
> > Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> > ---
> 
> Jingoo, could you copy-pastebin your config file. I don't have this
> warning and I would like to understand why.

Hi Daniel Lezcano,

Could you build the code by using GCC 4.6.x?
In my opinion, it would be better.

Also, my config option is as below:
    make exynos4_defconfig + CONFIG_CPU_IDLE

Best regards,
Jingoo Han


> 
> Thanks
>   -- Daniel
> 
> >  drivers/cpuidle/cpuidle.h |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/cpuidle/cpuidle.h b/drivers/cpuidle/cpuidle.h
> > index a5bbd1c..2120d9e 100644
> > --- a/drivers/cpuidle/cpuidle.h
> > +++ b/drivers/cpuidle/cpuidle.h
> > @@ -5,6 +5,8 @@
> >  #ifndef __DRIVER_CPUIDLE_H
> >  #define __DRIVER_CPUIDLE_H
> >
> > +#include <linux/device.h>
> > +
> >  /* For internal use only */
> >  extern struct cpuidle_governor *cpuidle_curr_governor;
> >  extern struct list_head cpuidle_governors;
> 
> 
> --
>  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
> 
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH] cpuidle: add missing header include

[Daniel Lezcano]

On 10/29/2012 02:16 AM, Jingoo Han wrote:
> On Monday, October 29, 2012 6:49 AM Marek Vasut wrote
>>
>> On 10/26/2012 06:30 AM, Jingoo Han wrote:
>>> This patch adds missing device.h header to fix build warnings as below:
>>>
>>> drivers/cpuidle/cpuidle.h:26:41: warning: 'struct device' declared inside parameter list [enabled by
>> default]
>>> drivers/cpuidle/cpuidle.h:26:41: warning: its scope is only this definition or declaration, which is
>> probably not what you want
>>> [enabled by default]
>>> drivers/cpuidle/cpuidle.h:27:45: warning: 'struct device' declared inside parameter list [enabled by
>> default]
>>> In file included from drivers/cpuidle/driver.c:15:0:
>>> drivers/cpuidle/cpuidle.h:26:41: warning: 'struct device' declared inside parameter list [enabled by
>> default]
>>> drivers/cpuidle/cpuidle.h:26:41: warning: its scope is only this definition or declaration, which is
>> probably not what you want
>>> [enabled by default]
>>> drivers/cpuidle/cpuidle.h:27:45: warning: 'struct device' declared inside parameter list [enabled by
>> default]
>>>
>>> This build warning is introduced by commit efeca1b
>>> "cpuidle / sysfs: change function parameter".
>>>
>>> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
>>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>>> ---
>>
>> Jingoo, could you copy-pastebin your config file. I don't have this
>> warning and I would like to understand why.
> 
> Hi Daniel Lezcano,
> 
> Could you build the code by using GCC 4.6.x?
> In my opinion, it would be better.

That was the case.

 => gcc (Ubuntu/Linaro 4.6.1-9ubuntu3) 4.6.1

> Also, my config option is as below:
>     make exynos4_defconfig + CONFIG_CPU_IDLE

Ok, I got the warning with this arch.

Let's see why this is not happening with x86 ...

Thanks
  -- Daniel

> Best regards,
> Jingoo Han
> 
> 
>>
>> Thanks
>>   -- Daniel
>>
>>>  drivers/cpuidle/cpuidle.h |    2 ++
>>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/cpuidle/cpuidle.h b/drivers/cpuidle/cpuidle.h
>>> index a5bbd1c..2120d9e 100644
>>> --- a/drivers/cpuidle/cpuidle.h
>>> +++ b/drivers/cpuidle/cpuidle.h
>>> @@ -5,6 +5,8 @@
>>>  #ifndef __DRIVER_CPUIDLE_H
>>>  #define __DRIVER_CPUIDLE_H
>>>
>>> +#include <linux/device.h>
>>> +
>>>  /* For internal use only */
>>>  extern struct cpuidle_governor *cpuidle_curr_governor;
>>>  extern struct list_head cpuidle_governors;
>>
>>
>> --
>>  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>>
>> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
>> <http://twitter.com/#!/linaroorg> Twitter |
>> <http://www.linaro.org/linaro-blog/> Blog
> 


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH] cpuidle: add missing header include

[Daniel Lezcano]

On 10/29/2012 02:16 AM, Jingoo Han wrote:
> On Monday, October 29, 2012 6:49 AM Marek Vasut wrote
>>
>> On 10/26/2012 06:30 AM, Jingoo Han wrote:
>>> This patch adds missing device.h header to fix build warnings as below:
>>>
>>> drivers/cpuidle/cpuidle.h:26:41: warning: 'struct device' declared inside parameter list [enabled by
>> default]
>>> drivers/cpuidle/cpuidle.h:26:41: warning: its scope is only this definition or declaration, which is
>> probably not what you want
>>> [enabled by default]
>>> drivers/cpuidle/cpuidle.h:27:45: warning: 'struct device' declared inside parameter list [enabled by
>> default]
>>> In file included from drivers/cpuidle/driver.c:15:0:
>>> drivers/cpuidle/cpuidle.h:26:41: warning: 'struct device' declared inside parameter list [enabled by
>> default]
>>> drivers/cpuidle/cpuidle.h:26:41: warning: its scope is only this definition or declaration, which is
>> probably not what you want
>>> [enabled by default]
>>> drivers/cpuidle/cpuidle.h:27:45: warning: 'struct device' declared inside parameter list [enabled by
>> default]
>>>
>>> This build warning is introduced by commit efeca1b
>>> "cpuidle / sysfs: change function parameter".
>>>
>>> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
>>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>>> ---
>>
>> Jingoo, could you copy-pastebin your config file. I don't have this
>> warning and I would like to understand why.
> 
> Hi Daniel Lezcano,
> 
> Could you build the code by using GCC 4.6.x?
> In my opinion, it would be better.
> 
> Also, my config option is as below:
>     make exynos4_defconfig + CONFIG_CPU_IDLE
> 

Ok, I got it.

on x86, the header ioport.h is included indirectly by the headers
including headers. In this file there is a forward declaration.
For ARM, this is not the case.

This is why the warning does not appear on x86 but on ARM.

Thanks
  -- Daniel

>> Thanks
>>   -- Daniel
>>
>>>  drivers/cpuidle/cpuidle.h |    2 ++
>>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/cpuidle/cpuidle.h b/drivers/cpuidle/cpuidle.h
>>> index a5bbd1c..2120d9e 100644
>>> --- a/drivers/cpuidle/cpuidle.h
>>> +++ b/drivers/cpuidle/cpuidle.h
>>> @@ -5,6 +5,8 @@
>>>  #ifndef __DRIVER_CPUIDLE_H
>>>  #define __DRIVER_CPUIDLE_H
>>>
>>> +#include <linux/device.h>
>>> +
>>>  /* For internal use only */
>>>  extern struct cpuidle_governor *cpuidle_curr_governor;
>>>  extern struct list_head cpuidle_governors;
>>
>>
>> --
>>  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>>
>> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
>> <http://twitter.com/#!/linaroorg> Twitter |
>> <http://www.linaro.org/linaro-blog/> Blog
> 


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog