| Message ID | 001401cdb332$93ffe450$bbffacf0$%han@samsung.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
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; >
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; >>
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;
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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 >
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 >
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;
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(-)