tlg2300: cleanups when power management is not configured

Message ID 4B8A7B83.8060203@freemail.hu (mailing list archive)
State Superseded, archived
Headers

Commit Message

Németh Márton Feb. 28, 2010, 2:19 p.m. UTC
  From: Márton Németh <nm127@freemail.hu>

When power management is not configured (CONFIG_PM) then some code is no longer
necessary.

This patch will remove the following compiler warnings:
 * pd-dvb.c: In function 'poseidon_fe_release':
 * pd-dvb.c:101: warning: unused variable 'pd'
 * pd-video.c:14: warning: 'pm_video_suspend' declared 'static' but never defined
 * pd-video.c:15: warning: 'pm_video_resume' declared 'static' but never defined

Signed-off-by: Márton Németh <nm127@freemail.hu>
---
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Comments

Huang Shijie March 1, 2010, 4:12 a.m. UTC | #1
hi Marton, thanks a lot.

> From: Márton Németh<nm127@freemail.hu>
>
> When power management is not configured (CONFIG_PM) then some code is no longer
> necessary.
>
> This patch will remove the following compiler warnings:
>   * pd-dvb.c: In function 'poseidon_fe_release':
>   * pd-dvb.c:101: warning: unused variable 'pd'
>   * pd-video.c:14: warning: 'pm_video_suspend' declared 'static' but never defined
>   * pd-video.c:15: warning: 'pm_video_resume' declared 'static' but never defined
>
> Signed-off-by: Márton Németh<nm127@freemail.hu>
> ---
> diff -r 37581bb7e6f1 linux/drivers/media/video/tlg2300/pd-dvb.c
> --- a/linux/drivers/media/video/tlg2300/pd-dvb.c	Wed Feb 24 22:48:50 2010 -0300
> +++ b/linux/drivers/media/video/tlg2300/pd-dvb.c	Sun Feb 28 15:13:05 2010 +0100
> @@ -96,15 +96,17 @@
>   	return ret;
>   }
>
> +#ifdef CONFIG_PM
>   static void poseidon_fe_release(struct dvb_frontend *fe)
>   {
>   	struct poseidon *pd = fe->demodulator_priv;
>
> -#ifdef CONFIG_PM
>   	pd->pm_suspend = NULL;
>   	pd->pm_resume  = NULL;
> +}
> +#else
> +#define poseidon_fe_release NULL
>   #endif
> -}
>
>    
I think the change here is a little more complicated.I prefer to change 
it like this :

static void poseidon_fe_release(struct dvb_frontend *fe)
{
#ifdef CONFIG_PM
     struct poseidon *pd = fe->demodulator_priv;
     pd->pm_suspend = NULL;
     pd->pm_resume  = NULL;
#endif
}

Could you change the patch, and resend it to me ?
thanks.
>   static s32 poseidon_fe_sleep(struct dvb_frontend *fe)
>   {
> diff -r 37581bb7e6f1 linux/drivers/media/video/tlg2300/pd-video.c
> --- a/linux/drivers/media/video/tlg2300/pd-video.c	Wed Feb 24 22:48:50 2010 -0300
> +++ b/linux/drivers/media/video/tlg2300/pd-video.c	Sun Feb 28 15:13:05 2010 +0100
> @@ -11,8 +11,10 @@
>   #include "pd-common.h"
>   #include "vendorcmds.h"
>
> +#ifdef CONFIG_PM
>   static int pm_video_suspend(struct poseidon *pd);
>   static int pm_video_resume(struct poseidon *pd);
> +#endif
>   static void iso_bubble_handler(struct work_struct *w);
>
>   int usb_transfer_mode;
>
>    

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Németh Márton March 1, 2010, 6:39 a.m. UTC | #2
Hi,
Huang Shijie wrote:
> hi Marton, thanks a lot.
> 
>> From: Márton Németh<nm127@freemail.hu>
>>
>> When power management is not configured (CONFIG_PM) then some code is no longer
>> necessary.
>>
>> This patch will remove the following compiler warnings:
>>   * pd-dvb.c: In function 'poseidon_fe_release':
>>   * pd-dvb.c:101: warning: unused variable 'pd'
>>   * pd-video.c:14: warning: 'pm_video_suspend' declared 'static' but never defined
>>   * pd-video.c:15: warning: 'pm_video_resume' declared 'static' but never defined
>>
>> Signed-off-by: Márton Németh<nm127@freemail.hu>
>> ---
>> diff -r 37581bb7e6f1 linux/drivers/media/video/tlg2300/pd-dvb.c
>> --- a/linux/drivers/media/video/tlg2300/pd-dvb.c	Wed Feb 24 22:48:50 2010 -0300
>> +++ b/linux/drivers/media/video/tlg2300/pd-dvb.c	Sun Feb 28 15:13:05 2010 +0100
>> @@ -96,15 +96,17 @@
>>   	return ret;
>>   }
>>
>> +#ifdef CONFIG_PM
>>   static void poseidon_fe_release(struct dvb_frontend *fe)
>>   {
>>   	struct poseidon *pd = fe->demodulator_priv;
>>
>> -#ifdef CONFIG_PM
>>   	pd->pm_suspend = NULL;
>>   	pd->pm_resume  = NULL;
>> +}
>> +#else
>> +#define poseidon_fe_release NULL
>>   #endif
>> -}
>>
>>    
> I think the change here is a little more complicated.I prefer to change 
> it like this :
> 
> static void poseidon_fe_release(struct dvb_frontend *fe)
> {
> #ifdef CONFIG_PM
>      struct poseidon *pd = fe->demodulator_priv;
>      pd->pm_suspend = NULL;
>      pd->pm_resume  = NULL;
> #endif
> }
> 
> Could you change the patch, and resend it to me ?
> thanks.

I'm afraid in this case we'll get a warning saying that the parameter fe is unused.
Here is an example:

	$ gcc -Wall -O2 -Wextra test.c
	test.c: In function ‘foo’:
	test.c:2: warning: unused parameter ‘x’
	$ cat test.c
	
	static void foo(int x)
	{
	
	}
	
	int main()
	{
		foo(0);
		return 0;
	}

The second reason I modified the code like this is that the the .release
opreation is used with the following sequence:

	if (dvb->frontend->ops.release)
		dvb->frontend->ops.release(dvb->frontend);

If power management is not configured then the symbol poseidon_fe_release will be
NULL and the condition dvb->frontend->ops.release will be false. So the otherwise
empty function will not be called at all.

Regards,

	Márton Németh
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Huang Shijie March 1, 2010, 7:54 a.m. UTC | #3
The function poseidon_fe_release is redundent in fact, I will remove it 
in future.

But now I still acked this patch. thanks a lot.

Acked-by: Huang Shijie <shijie8@gmail.com>
> From: Márton Németh<nm127@freemail.hu>
>
> When power management is not configured (CONFIG_PM) then some code is no longer
> necessary.
>
> This patch will remove the following compiler warnings:
>   * pd-dvb.c: In function 'poseidon_fe_release':
>   * pd-dvb.c:101: warning: unused variable 'pd'
>   * pd-video.c:14: warning: 'pm_video_suspend' declared 'static' but never defined
>   * pd-video.c:15: warning: 'pm_video_resume' declared 'static' but never defined
>
> Signed-off-by: Márton Németh<nm127@freemail.hu>
> ---
> diff -r 37581bb7e6f1 linux/drivers/media/video/tlg2300/pd-dvb.c
> --- a/linux/drivers/media/video/tlg2300/pd-dvb.c	Wed Feb 24 22:48:50 2010 -0300
> +++ b/linux/drivers/media/video/tlg2300/pd-dvb.c	Sun Feb 28 15:13:05 2010 +0100
> @@ -96,15 +96,17 @@
>   	return ret;
>   }
>
> +#ifdef CONFIG_PM
>   static void poseidon_fe_release(struct dvb_frontend *fe)
>   {
>   	struct poseidon *pd = fe->demodulator_priv;
>
> -#ifdef CONFIG_PM
>   	pd->pm_suspend = NULL;
>   	pd->pm_resume  = NULL;
> +}
> +#else
> +#define poseidon_fe_release NULL
>   #endif
> -}
>
>   static s32 poseidon_fe_sleep(struct dvb_frontend *fe)
>   {
> diff -r 37581bb7e6f1 linux/drivers/media/video/tlg2300/pd-video.c
> --- a/linux/drivers/media/video/tlg2300/pd-video.c	Wed Feb 24 22:48:50 2010 -0300
> +++ b/linux/drivers/media/video/tlg2300/pd-video.c	Sun Feb 28 15:13:05 2010 +0100
> @@ -11,8 +11,10 @@
>   #include "pd-common.h"
>   #include "vendorcmds.h"
>
> +#ifdef CONFIG_PM
>   static int pm_video_suspend(struct poseidon *pd);
>   static int pm_video_resume(struct poseidon *pd);
> +#endif
>   static void iso_bubble_handler(struct work_struct *w);
>
>   int usb_transfer_mode;
>
>    

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Patch

diff -r 37581bb7e6f1 linux/drivers/media/video/tlg2300/pd-dvb.c
--- a/linux/drivers/media/video/tlg2300/pd-dvb.c	Wed Feb 24 22:48:50 2010 -0300
+++ b/linux/drivers/media/video/tlg2300/pd-dvb.c	Sun Feb 28 15:13:05 2010 +0100
@@ -96,15 +96,17 @@ 
 	return ret;
 }

+#ifdef CONFIG_PM
 static void poseidon_fe_release(struct dvb_frontend *fe)
 {
 	struct poseidon *pd = fe->demodulator_priv;

-#ifdef CONFIG_PM
 	pd->pm_suspend = NULL;
 	pd->pm_resume  = NULL;
+}
+#else
+#define poseidon_fe_release NULL
 #endif
-}

 static s32 poseidon_fe_sleep(struct dvb_frontend *fe)
 {
diff -r 37581bb7e6f1 linux/drivers/media/video/tlg2300/pd-video.c
--- a/linux/drivers/media/video/tlg2300/pd-video.c	Wed Feb 24 22:48:50 2010 -0300
+++ b/linux/drivers/media/video/tlg2300/pd-video.c	Sun Feb 28 15:13:05 2010 +0100
@@ -11,8 +11,10 @@ 
 #include "pd-common.h"
 #include "vendorcmds.h"

+#ifdef CONFIG_PM
 static int pm_video_suspend(struct poseidon *pd);
 static int pm_video_resume(struct poseidon *pd);
+#endif
 static void iso_bubble_handler(struct work_struct *w);

 int usb_transfer_mode;