* [PATCH] cpuidle: add missing header include @ 2012-10-26 4:30 Jingoo Han 2012-10-26 11:14 ` Rafael J. Wysocki 2012-10-28 21:49 ` Daniel Lezcano 0 siblings, 2 replies; 7+ messages in thread From: Jingoo Han @ 2012-10-26 4:30 UTC (permalink / raw) To: 'Rafael J. Wysocki' Cc: linux-pm, linux-kernel, 'Daniel Lezcano', '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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] cpuidle: add missing header include 2012-10-26 4:30 [PATCH] cpuidle: add missing header include Jingoo Han @ 2012-10-26 11:14 ` Rafael J. Wysocki 2012-10-26 12:45 ` Daniel Lezcano 2012-10-28 21:49 ` Daniel Lezcano 1 sibling, 1 reply; 7+ messages in thread From: Rafael J. Wysocki @ 2012-10-26 11:14 UTC (permalink / raw) To: Jingoo Han, 'Daniel Lezcano' Cc: 'Rafael J. Wysocki', linux-pm, linux-kernel 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] cpuidle: add missing header include 2012-10-26 11:14 ` Rafael J. Wysocki @ 2012-10-26 12:45 ` Daniel Lezcano 0 siblings, 0 replies; 7+ messages in thread From: Daniel Lezcano @ 2012-10-26 12:45 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Jingoo Han, 'Rafael J. Wysocki', linux-pm, linux-kernel 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] cpuidle: add missing header include 2012-10-26 4:30 [PATCH] cpuidle: add missing header include Jingoo Han 2012-10-26 11:14 ` Rafael J. Wysocki @ 2012-10-28 21:49 ` Daniel Lezcano 2012-10-29 1:16 ` Jingoo Han 1 sibling, 1 reply; 7+ messages in thread From: Daniel Lezcano @ 2012-10-28 21:49 UTC (permalink / raw) To: Jingoo Han; +Cc: 'Rafael J. Wysocki', linux-pm, linux-kernel 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] cpuidle: add missing header include 2012-10-28 21:49 ` Daniel Lezcano @ 2012-10-29 1:16 ` Jingoo Han 2012-10-29 6:27 ` Daniel Lezcano 2012-10-29 9:45 ` Daniel Lezcano 0 siblings, 2 replies; 7+ messages in thread From: Jingoo Han @ 2012-10-29 1:16 UTC (permalink / raw) To: 'Daniel Lezcano' Cc: 'Rafael J. Wysocki', linux-pm, linux-kernel, '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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] cpuidle: add missing header include 2012-10-29 1:16 ` Jingoo Han @ 2012-10-29 6:27 ` Daniel Lezcano 2012-10-29 9:45 ` Daniel Lezcano 1 sibling, 0 replies; 7+ messages in thread From: Daniel Lezcano @ 2012-10-29 6:27 UTC (permalink / raw) To: Jingoo Han; +Cc: 'Rafael J. Wysocki', linux-pm, linux-kernel 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] cpuidle: add missing header include 2012-10-29 1:16 ` Jingoo Han 2012-10-29 6:27 ` Daniel Lezcano @ 2012-10-29 9:45 ` Daniel Lezcano 1 sibling, 0 replies; 7+ messages in thread From: Daniel Lezcano @ 2012-10-29 9:45 UTC (permalink / raw) To: Jingoo Han; +Cc: 'Rafael J. Wysocki', linux-pm, linux-kernel 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-10-29 9:46 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-10-26 4:30 [PATCH] cpuidle: add missing header include Jingoo Han 2012-10-26 11:14 ` Rafael J. Wysocki 2012-10-26 12:45 ` Daniel Lezcano 2012-10-28 21:49 ` Daniel Lezcano 2012-10-29 1:16 ` Jingoo Han 2012-10-29 6:27 ` Daniel Lezcano 2012-10-29 9:45 ` Daniel Lezcano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).