tlg2300: cleanups when power management is not configured

PATCH In review Németh Márton 2 participants 4 messages Feb 28, 2010 – Mar 1, 2010 Media (V4L/DVB)

lore

Share & tools

lore

Németh Márton

author

2010-02-28 14:20 UTC #0 lore raw

PATCH 1/1 tlg2300: cleanups when power management is not configured

From: Márton Németh <[email protected]>

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 <[email protected]>

---

linux/drivers/media/video/tlg2300/pd-dvb.c +4 -2

diff -r 37581bb7e6f1 linux/drivers/media/video/tlg2300/pd-dvb.c
@@ -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)
 {

linux/drivers/media/video/tlg2300/pd-video.c +2 -0

diff -r 37581bb7e6f1 linux/drivers/media/video/tlg2300/pd-video.c
@@ -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;

Huang Shijie

2010-03-01 04:12 UTC #1 lore raw

hi Marton, thanks a lot.

Quoted 25 lines

From: Márton Németh<[email protected]>
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<[email protected]>
---
diff -r 37581bb7e6f1 linux/drivers/media/video/tlg2300/pd-dvb.c
linux/drivers/media/video/tlg2300/pd-dvb.c +1 -0
@@ -96,15 +96,17 @@
  	return ret;
  }
 
+#ifdef CONFIG_PM
  static void poseidon_fe_release(struct dvb_frontend *fe)
  {
  	struct poseidon *pd = fe->demodulator_priv;
 

linux/drivers/media/video/tlg2300/pd-dvb.c line 111

Quoted 10 lines

linux/drivers/media/video/tlg2300/pd-dvb.c +3 -2

-#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.

Quoted 18 lines

linux/drivers/media/video/tlg2300/pd-dvb.c +0 -0

  static s32 poseidon_fe_sleep(struct dvb_frontend *fe)

{
diff -r 37581bb7e6f1 linux/drivers/media/video/tlg2300/pd-video.c

linux/drivers/media/video/tlg2300/pd-video.c +2 -0

@@ -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;

Németh Márton

author

2010-03-01 06:39 UTC #2 lore raw

Hi,

Quoted 46 lines · Huang Shijie

hi Marton, thanks a lot.
Quoted 25 lines
From: Márton Németh<[email protected]>
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<[email protected]>
---
diff -r 37581bb7e6f1 linux/drivers/media/video/tlg2300/pd-dvb.c
linux/drivers/media/video/tlg2300/pd-dvb.c +1 -0
@@ -96,15 +96,17 @@
  	return ret;
  }
 
+#ifdef CONFIG_PM
  static void poseidon_fe_release(struct dvb_frontend *fe)
  {
  	struct poseidon *pd = fe->demodulator_priv;
 
linux/drivers/media/video/tlg2300/pd-dvb.c line 111
Quoted 10 lines
linux/drivers/media/video/tlg2300/pd-dvb.c +3 -2
-#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;

Quoted 5 lines

#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

Huang Shijie

Acked-by on v1 1/1

2010-03-01 07:54 UTC #3 lore raw

In reply to Németh Márton

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 <[email protected]>

Quoted 52 lines

From: Márton Németh<[email protected]>
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<[email protected]>
---
diff -r 37581bb7e6f1 linux/drivers/media/video/tlg2300/pd-dvb.c
linux/drivers/media/video/tlg2300/pd-dvb.c +4 -2
@@ -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
linux/drivers/media/video/tlg2300/pd-video.c +2 -0
@@ -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;