| Message ID | 000801cff1e2$2427ba40$6c772ec0$%han@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Jingoo, On Mon, Oct 27, 2014 at 09:32:58PM +0900, Jingoo Han wrote: > Add CONFIG_PM_SLEEP to suspend/resume functions > > Add CONFIG_PM_SLEEP to suspend/resume functions instead of > CONFIG_PM to fix the following build warning when CONFIG_PM_SLEEP > is not selected and CONFIG_PM is selected. This is because sleep > PM callbacks defined by SIMPLE_DEV_PM_OPS are only used when the > CONFIG_PM_SLEEP is enabled. Recently I've become a fan of __maybe_unused markings as they insulate us from various CONFIG changes in unrelated subsystems, I'll transform this patch to use them instead. Thanks. > > drivers/input/keyboard/spear-keyboard.c:292:12: warning: 'spear_kbd_suspend' defined but not used [-Wunused-function] > drivers/input/keyboard/spear-keyboard.c:345:12: warning: 'spear_kbd_resume' defined but not used [-Wunused-function] > > Signed-off-by: Jingoo Han <jg1.han@samsung.com> > --- > drivers/input/keyboard/spear-keyboard.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/input/keyboard/spear-keyboard.c b/drivers/input/keyboard/spear-keyboard.c > index 258af10..fc93e79 100644 > --- a/drivers/input/keyboard/spear-keyboard.c > +++ b/drivers/input/keyboard/spear-keyboard.c > @@ -288,7 +288,7 @@ static int spear_kbd_remove(struct platform_device *pdev) > return 0; > } > > -#ifdef CONFIG_PM > +#ifdef CONFIG_PM_SLEEP > static int spear_kbd_suspend(struct device *dev) > { > struct platform_device *pdev = to_platform_device(dev); > -- > 1.7.9.5 > >
On Tuesday, October 28, 2014 8:38 AM, Dmitry Torokhov wrote: > On Mon, Oct 27, 2014 at 09:32:58PM +0900, Jingoo Han wrote: > > Add CONFIG_PM_SLEEP to suspend/resume functions > > > > Add CONFIG_PM_SLEEP to suspend/resume functions instead of > > CONFIG_PM to fix the following build warning when CONFIG_PM_SLEEP > > is not selected and CONFIG_PM is selected. This is because sleep > > PM callbacks defined by SIMPLE_DEV_PM_OPS are only used when the > > CONFIG_PM_SLEEP is enabled. > > Recently I've become a fan of __maybe_unused markings as they insulate > us from various CONFIG changes in unrelated subsystems, I'll transform > this patch to use them instead. OK, I see. I have no objection. Then, how about changing other usages of CONFIG_PM_SLEEP/CONFIG_PM to __maybe_unused annotation? Personally, I prefer to increase build coverage than using #ifdef guards. Someone, however, argued that #ifdef guards should be used in this case because the size of binary can be reduced. How about your opinion? Thank you. Best regards, Jingoo Han > > Thanks. > > > > > drivers/input/keyboard/spear-keyboard.c:292:12: warning: 'spear_kbd_suspend' defined but not used [- > Wunused-function] > > drivers/input/keyboard/spear-keyboard.c:345:12: warning: 'spear_kbd_resume' defined but not used [- > Wunused-function] > > > > Signed-off-by: Jingoo Han <jg1.han@samsung.com> > > --- > > drivers/input/keyboard/spear-keyboard.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/input/keyboard/spear-keyboard.c b/drivers/input/keyboard/spear-keyboard.c > > index 258af10..fc93e79 100644 > > --- a/drivers/input/keyboard/spear-keyboard.c > > +++ b/drivers/input/keyboard/spear-keyboard.c > > @@ -288,7 +288,7 @@ static int spear_kbd_remove(struct platform_device *pdev) > > return 0; > > } > > > > -#ifdef CONFIG_PM > > +#ifdef CONFIG_PM_SLEEP > > static int spear_kbd_suspend(struct device *dev) > > { > > struct platform_device *pdev = to_platform_device(dev); > > -- > > 1.7.9.5 > > > > > > -- > Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Oct 28, 2014 at 09:02:44AM +0900, Jingoo Han wrote: > On Tuesday, October 28, 2014 8:38 AM, Dmitry Torokhov wrote: > > On Mon, Oct 27, 2014 at 09:32:58PM +0900, Jingoo Han wrote: > > > Add CONFIG_PM_SLEEP to suspend/resume functions > > > > > > Add CONFIG_PM_SLEEP to suspend/resume functions instead of > > > CONFIG_PM to fix the following build warning when CONFIG_PM_SLEEP > > > is not selected and CONFIG_PM is selected. This is because sleep > > > PM callbacks defined by SIMPLE_DEV_PM_OPS are only used when the > > > CONFIG_PM_SLEEP is enabled. > > > > Recently I've become a fan of __maybe_unused markings as they insulate > > us from various CONFIG changes in unrelated subsystems, I'll transform > > this patch to use them instead. > > OK, I see. I have no objection. > Then, how about changing other usages of CONFIG_PM_SLEEP/CONFIG_PM > to __maybe_unused annotation? Personally, I prefer to increase build > coverage than using #ifdef guards. Someone, however, argued that #ifdef > guards should be used in this case because the size of binary can be > reduced. How about your opinion? The optimizer is supposed to drop functions marked as '__maybe_unused' if they are indeed unused so size of the binary should not change. Thanks.
On Tuesday, October 28, 2014 9:11 AM, Dmitry Torokhov wrote: > On Tue, Oct 28, 2014 at 09:02:44AM +0900, Jingoo Han wrote: > > On Tuesday, October 28, 2014 8:38 AM, Dmitry Torokhov wrote: > > > On Mon, Oct 27, 2014 at 09:32:58PM +0900, Jingoo Han wrote: > > > > Add CONFIG_PM_SLEEP to suspend/resume functions > > > > > > > > Add CONFIG_PM_SLEEP to suspend/resume functions instead of > > > > CONFIG_PM to fix the following build warning when CONFIG_PM_SLEEP > > > > is not selected and CONFIG_PM is selected. This is because sleep > > > > PM callbacks defined by SIMPLE_DEV_PM_OPS are only used when the > > > > CONFIG_PM_SLEEP is enabled. > > > > > > Recently I've become a fan of __maybe_unused markings as they insulate > > > us from various CONFIG changes in unrelated subsystems, I'll transform > > > this patch to use them instead. > > > > OK, I see. I have no objection. > > Then, how about changing other usages of CONFIG_PM_SLEEP/CONFIG_PM > > to __maybe_unused annotation? Personally, I prefer to increase build > > coverage than using #ifdef guards. Someone, however, argued that #ifdef > > guards should be used in this case because the size of binary can be > > reduced. How about your opinion? > > The optimizer is supposed to drop functions marked as '__maybe_unused' > if they are indeed unused so size of the binary should not change. Oh, I really appreciate your reply. :-) Then, how about changing other usages of CONFIG_PM_SLEEP/CONFIG_PM to __maybe_unused annotation? If you don't have any objection, I will send one single patch for this. Best regards, Jingoo Han > > Thanks. > > -- > Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday, October 28, 2014 09:34:06 AM Jingoo Han wrote: > On Tuesday, October 28, 2014 9:11 AM, Dmitry Torokhov wrote: > > On Tue, Oct 28, 2014 at 09:02:44AM +0900, Jingoo Han wrote: > > > On Tuesday, October 28, 2014 8:38 AM, Dmitry Torokhov wrote: > > > > On Mon, Oct 27, 2014 at 09:32:58PM +0900, Jingoo Han wrote: > > > > > Add CONFIG_PM_SLEEP to suspend/resume functions > > > > > > > > > > Add CONFIG_PM_SLEEP to suspend/resume functions instead of > > > > > CONFIG_PM to fix the following build warning when CONFIG_PM_SLEEP > > > > > is not selected and CONFIG_PM is selected. This is because sleep > > > > > PM callbacks defined by SIMPLE_DEV_PM_OPS are only used when the > > > > > CONFIG_PM_SLEEP is enabled. > > > > > > > > Recently I've become a fan of __maybe_unused markings as they insulate > > > > us from various CONFIG changes in unrelated subsystems, I'll transform > > > > this patch to use them instead. > > > > > > OK, I see. I have no objection. > > > Then, how about changing other usages of CONFIG_PM_SLEEP/CONFIG_PM > > > to __maybe_unused annotation? Personally, I prefer to increase build > > > coverage than using #ifdef guards. Someone, however, argued that #ifdef > > > guards should be used in this case because the size of binary can be > > > reduced. How about your opinion? > > > > The optimizer is supposed to drop functions marked as '__maybe_unused' > > if they are indeed unused so size of the binary should not change. > > Oh, I really appreciate your reply. :-) > Then, how about changing other usages of CONFIG_PM_SLEEP/CONFIG_PM > to __maybe_unused annotation? If you don't have any objection, I will > send one single patch for this. That will work. Thanks.
On Tuesday, October 28, 2014 9:11 AM, Dmitry Torokhov wrote: > On Tue, Oct 28, 2014 at 09:02:44AM +0900, Jingoo Han wrote: > > On Tuesday, October 28, 2014 8:38 AM, Dmitry Torokhov wrote: > > > On Mon, Oct 27, 2014 at 09:32:58PM +0900, Jingoo Han wrote: > > > > Add CONFIG_PM_SLEEP to suspend/resume functions > > > > > > > > Add CONFIG_PM_SLEEP to suspend/resume functions instead of > > > > CONFIG_PM to fix the following build warning when CONFIG_PM_SLEEP > > > > is not selected and CONFIG_PM is selected. This is because sleep > > > > PM callbacks defined by SIMPLE_DEV_PM_OPS are only used when the > > > > CONFIG_PM_SLEEP is enabled. > > > > > > Recently I've become a fan of __maybe_unused markings as they insulate > > > us from various CONFIG changes in unrelated subsystems, I'll transform > > > this patch to use them instead. > > > > OK, I see. I have no objection. > > Then, how about changing other usages of CONFIG_PM_SLEEP/CONFIG_PM > > to __maybe_unused annotation? Personally, I prefer to increase build > > coverage than using #ifdef guards. Someone, however, argued that #ifdef > > guards should be used in this case because the size of binary can be > > reduced. How about your opinion? > > The optimizer is supposed to drop functions marked as '__maybe_unused' > if they are indeed unused so size of the binary should not change. Sorry for annoying you. I built 'spear-keyboard' and got the binaries as below. spear-keyboard.o 89500 bytes, <-- CONFIG_PM_SLEEP=n spear-keyboard.o 92352 bytes, <-- CONFIG_PM_SLEEP=y spear-keyboard.o 92352 bytes, <-- Marked as __maybe_unused As presented above, when CONFIG_PM_SLEEP is used instead of __maybe_unused, the size of binary is reduced. So, someone complained me to use #ifdef guards. But, I agree with your opinion. Personally, I DON'T want to use #ifdef guards, because I prefer to increase build coverage. Thank you. > > Thanks. > > -- > Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On October 27, 2014 5:53:56 PM PDT, Jingoo Han <jg1.han@samsung.com> wrote: >On Tuesday, October 28, 2014 9:11 AM, Dmitry Torokhov wrote: >> On Tue, Oct 28, 2014 at 09:02:44AM +0900, Jingoo Han wrote: >> > On Tuesday, October 28, 2014 8:38 AM, Dmitry Torokhov wrote: >> > > On Mon, Oct 27, 2014 at 09:32:58PM +0900, Jingoo Han wrote: >> > > > Add CONFIG_PM_SLEEP to suspend/resume functions >> > > > >> > > > Add CONFIG_PM_SLEEP to suspend/resume functions instead of >> > > > CONFIG_PM to fix the following build warning when >CONFIG_PM_SLEEP >> > > > is not selected and CONFIG_PM is selected. This is because >sleep >> > > > PM callbacks defined by SIMPLE_DEV_PM_OPS are only used when >the >> > > > CONFIG_PM_SLEEP is enabled. >> > > >> > > Recently I've become a fan of __maybe_unused markings as they >insulate >> > > us from various CONFIG changes in unrelated subsystems, I'll >transform >> > > this patch to use them instead. >> > >> > OK, I see. I have no objection. >> > Then, how about changing other usages of CONFIG_PM_SLEEP/CONFIG_PM >> > to __maybe_unused annotation? Personally, I prefer to increase >build >> > coverage than using #ifdef guards. Someone, however, argued that >#ifdef >> > guards should be used in this case because the size of binary can >be >> > reduced. How about your opinion? >> >> The optimizer is supposed to drop functions marked as >'__maybe_unused' >> if they are indeed unused so size of the binary should not change. > >Sorry for annoying you. >I built 'spear-keyboard' and got the binaries as below. > >spear-keyboard.o 89500 bytes, <-- CONFIG_PM_SLEEP=n >spear-keyboard.o 92352 bytes, <-- CONFIG_PM_SLEEP=y >spear-keyboard.o 92352 bytes, <-- Marked as __maybe_unused > >As presented above, when CONFIG_PM_SLEEP is used instead of >__maybe_unused, the size of binary is reduced. So, someone >complained me to use #ifdef guards. But, I agree with your >opinion. Personally, I DON'T want to use #ifdef guards, >because I prefer to increase build coverage. What about the final kernel image size/kernel module size? Thanks.
> -----Original Message----- > From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com] > Sent: Tuesday, October 28, 2014 10:09 AM > To: Jingoo Han > Cc: linux-input@vger.kernel.org; 'Viresh Kumar'; 'Jingoo Han' > Subject: Re: [PATCH] Input: spear-keyboard - Add CONFIG_PM_SLEEP to suspend/resume functions > > On October 27, 2014 5:53:56 PM PDT, Jingoo Han <jg1.han@samsung.com> wrote: > >On Tuesday, October 28, 2014 9:11 AM, Dmitry Torokhov wrote: > >> On Tue, Oct 28, 2014 at 09:02:44AM +0900, Jingoo Han wrote: > >> > On Tuesday, October 28, 2014 8:38 AM, Dmitry Torokhov wrote: > >> > > On Mon, Oct 27, 2014 at 09:32:58PM +0900, Jingoo Han wrote: > >> > > > Add CONFIG_PM_SLEEP to suspend/resume functions > >> > > > > >> > > > Add CONFIG_PM_SLEEP to suspend/resume functions instead of > >> > > > CONFIG_PM to fix the following build warning when > >CONFIG_PM_SLEEP > >> > > > is not selected and CONFIG_PM is selected. This is because > >sleep > >> > > > PM callbacks defined by SIMPLE_DEV_PM_OPS are only used when > >the > >> > > > CONFIG_PM_SLEEP is enabled. > >> > > > >> > > Recently I've become a fan of __maybe_unused markings as they > >insulate > >> > > us from various CONFIG changes in unrelated subsystems, I'll > >transform > >> > > this patch to use them instead. > >> > > >> > OK, I see. I have no objection. > >> > Then, how about changing other usages of CONFIG_PM_SLEEP/CONFIG_PM > >> > to __maybe_unused annotation? Personally, I prefer to increase > >build > >> > coverage than using #ifdef guards. Someone, however, argued that > >#ifdef > >> > guards should be used in this case because the size of binary can > >be > >> > reduced. How about your opinion? > >> > >> The optimizer is supposed to drop functions marked as > >'__maybe_unused' > >> if they are indeed unused so size of the binary should not change. > > > >Sorry for annoying you. > >I built 'spear-keyboard' and got the binaries as below. > > > >spear-keyboard.o 89500 bytes, <-- CONFIG_PM_SLEEP=n > >spear-keyboard.o 92352 bytes, <-- CONFIG_PM_SLEEP=y > >spear-keyboard.o 92352 bytes, <-- Marked as __maybe_unused > > > >As presented above, when CONFIG_PM_SLEEP is used instead of > >__maybe_unused, the size of binary is reduced. So, someone > >complained me to use #ifdef guards. But, I agree with your > >opinion. Personally, I DON'T want to use #ifdef guards, > >because I prefer to increase build coverage. > > What about the final kernel image size/kernel module size? I used 'spear13xx_defconfig' with linux-next(20141027). Then, I changed '#ifdef CONFIG_PM' into '#ifdef CONFIG_PM_SLEEP' as below. -#ifdef CONFIG_PM +#ifdef CONFIG_PM_SLEEP static int spear_kbd_suspend(struct device *dev) { Then, I got the following kernel images. 1. CONFIG_PM_SLEEP=y zImage 2891128 bytes spear-keyboard.o 88528 bytes 2. CONFIG_PM_SLEEP=n zImage 2842656 bytes spear-keyboard.o 85980 bytes 3. Marked as '__maybe_unused' for 'spear_kbd_suspend()/spear_kbd_resume()' instead of using '#ifdef CONFIG_PM_SLEEP', and CONFIG_PM_SLEEP=y zImage 2891160 bytes spear-keyboard.o 88528 bytes 4. Marked as '__maybe_unused' for 'spear_kbd_suspend()/spear_kbd_resume()' instead of using '#ifdef CONFIG_PM_SLEEP', and CONFIG_PM_SLEEP=n zImage 2842712 bytes spear-keyboard.o 85980 bytes Oh, sorry! Your comment is right. As you mentioned, the total sizes of the kernel and spear-keyboard.o are reduced, when '__maybe_unused' annotation is used. Thank you! Best regards, Jingoo Han > > > Thanks. > > -- > Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/input/keyboard/spear-keyboard.c b/drivers/input/keyboard/spear-keyboard.c index 258af10..fc93e79 100644 --- a/drivers/input/keyboard/spear-keyboard.c +++ b/drivers/input/keyboard/spear-keyboard.c @@ -288,7 +288,7 @@ static int spear_kbd_remove(struct platform_device *pdev) return 0; } -#ifdef CONFIG_PM +#ifdef CONFIG_PM_SLEEP static int spear_kbd_suspend(struct device *dev) { struct platform_device *pdev = to_platform_device(dev);
Add CONFIG_PM_SLEEP to suspend/resume functions Add CONFIG_PM_SLEEP to suspend/resume functions instead of CONFIG_PM to fix the following build warning when CONFIG_PM_SLEEP is not selected and CONFIG_PM is selected. This is because sleep PM callbacks defined by SIMPLE_DEV_PM_OPS are only used when the CONFIG_PM_SLEEP is enabled. drivers/input/keyboard/spear-keyboard.c:292:12: warning: 'spear_kbd_suspend' defined but not used [-Wunused-function] drivers/input/keyboard/spear-keyboard.c:345:12: warning: 'spear_kbd_resume' defined but not used [-Wunused-function] Signed-off-by: Jingoo Han <jg1.han@samsung.com> --- drivers/input/keyboard/spear-keyboard.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)