Re: [PATCH] cx88: convert core-tvaudio into an enum

2010-08-26 Thread Marton Balint 


On Mon, 23 Aug 2010, lawrence rust wrote:


Using an enum and removing the default case from switch statements accessing
the value enables the compiler to emit a warning (enabled with -Wall) when an
audio mode is not handled.

This highlights an omission in the function cx88_dsp_detect_stereo_sap()
(in cx88-dsp.c) not handling WW_EIAJ and WW_M.

Signed-off-by: Lawrence Rust lawre...@softsystem.co.uk
---
drivers/media/video/cx88/cx88-core.c|2 +-
drivers/media/video/cx88/cx88-dsp.c |   17 +-
drivers/media/video/cx88/cx88-tvaudio.c |   37 ++
drivers/media/video/cx88/cx88.h |   28 ---
4 files changed, 64 insertions(+), 20 deletions(-)

diff --git a/drivers/media/video/cx88/cx88-core.c 
b/drivers/media/video/cx88/cx88-core.c
index 8b21457..b0cf307 100644
--- a/drivers/media/video/cx88/cx88-core.c
+++ b/drivers/media/video/cx88/cx88-core.c
@@ -879,7 +879,7 @@ static int set_tvaudio(struct cx88_core *core)
} else {
printk(%s/0: tvaudio support needs work for this tv norm [%s], 
sorry\n,
   core-name, v4l2_norm_to_name(core-tvnorm));
-   core-tvaudio = 0;
+   core-tvaudio = WW_NONE;
return 0;
}

diff --git a/drivers/media/video/cx88/cx88-dsp.c 
b/drivers/media/video/cx88/cx88-dsp.c
index a94e00a..e1d6eef 100644
--- a/drivers/media/video/cx88/cx88-dsp.c
+++ b/drivers/media/video/cx88/cx88-dsp.c
@@ -175,7 +175,13 @@ static s32 detect_a2_a2m_eiaj(struct cx88_core *core, s16 
x[], u32 N)
stereo_freq = FREQ_EIAJ_STEREO;
dual_freq = FREQ_EIAJ_DUAL;
break;
-   default:
+   case WW_NONE:
+   case WW_BTSC:
+   case WW_I:
+   case WW_L:
+   case WW_I2SPT:
+   case WW_FM:
+   case WW_I2SADC:
printk(KERN_WARNING %s/0: unsupported audio mode %d for %s\n,
   core-name, core-tvaudio, __func__);
return UNSET;
@@ -292,11 +298,20 @@ s32 cx88_dsp_detect_stereo_sap(struct cx88_core *core)
switch (core-tvaudio) {
case WW_BG:
case WW_DK:
+   case WW_EIAJ:
+   case WW_M:


Those two lines were deliberately omitted because I could not test EIAJ 
and A2M modes. So unless someone with that type of signal can test and fix 
them, I think it would better to put those signal types into last part of 
of the switch statement with a 'Testing required' comment.


Regards,
  Marton



ret = detect_a2_a2m_eiaj(core, samples, N);
break;
case WW_BTSC:
ret = detect_btsc(core, samples, N);
break;
+   case WW_NONE:
+   case WW_I:
+   case WW_L:
+   case WW_I2SPT:
+   case WW_FM:
+   case WW_I2SADC:
+   break;
}

kfree(samples);
diff --git a/drivers/media/video/cx88/cx88-tvaudio.c 
b/drivers/media/video/cx88/cx88-tvaudio.c
index 2396315..db63547 100644
--- a/drivers/media/video/cx88/cx88-tvaudio.c
+++ b/drivers/media/video/cx88/cx88-tvaudio.c
@@ -360,7 +360,15 @@ static void set_audio_standard_NICAM(struct cx88_core 
*core, u32 mode)
set_audio_registers(core, nicam_bgdki_common);
set_audio_registers(core, nicam_i);
break;
-   default:
+   case WW_NONE:
+   case WW_BTSC:
+   case WW_BG:
+   case WW_DK:
+   case WW_EIAJ:
+   case WW_I2SPT:
+   case WW_FM:
+   case WW_I2SADC:
+   case WW_M:
dprintk(%s PAL-BGDK NICAM (status: known-good)\n, __func__);
set_audio_registers(core, nicam_bgdki_common);
set_audio_registers(core, nicam_default);
@@ -621,7 +629,13 @@ static void set_audio_standard_A2(struct cx88_core *core, 
u32 mode)
dprintk(%s AM-L (status: devel)\n, __func__);
set_audio_registers(core, am_l);
break;
-   default:
+   case WW_NONE:
+   case WW_BTSC:
+   case WW_EIAJ:
+   case WW_I2SPT:
+   case WW_FM:
+   case WW_I2SADC:
+   case WW_M:
dprintk(%s Warning: wrong value\n, __func__);
return;
break;
@@ -779,7 +793,7 @@ void cx88_set_tvaudio(struct cx88_core *core)
set_audio_finish(core, EN_I2SIN_ENABLE);
break;
case WW_NONE:
-   default:
+   case WW_I2SPT:
printk(%s/0: unknown tv audio mode [%d]\n,
   core-name, core-tvaudio);
break;
@@ -840,7 +854,12 @@ void cx88_get_stereo(struct cx88_core *core, struct 
v4l2_tuner *t)
break;
}
break;
-   default:
+   case WW_NONE:
+   case WW_I:
+   case WW_L:
+   case WW_I2SPT:
+   case WW_FM:
+   case WW_I2SADC:
/* nothing */
break;
}
@@ -945,6 +964,9 @@ void cx88_set_stereo(struct

[PATCH] cx88: convert core-tvaudio into an enum

2010-08-23 Thread lawrence rust 
Using an enum and removing the default case from switch statements accessing
the value enables the compiler to emit a warning (enabled with -Wall) when an
audio mode is not handled.

This highlights an omission in the function cx88_dsp_detect_stereo_sap()
(in cx88-dsp.c) not handling WW_EIAJ and WW_M.

Signed-off-by: Lawrence Rust lawre...@softsystem.co.uk
---
 drivers/media/video/cx88/cx88-core.c|2 +-
 drivers/media/video/cx88/cx88-dsp.c |   17 +-
 drivers/media/video/cx88/cx88-tvaudio.c |   37 ++
 drivers/media/video/cx88/cx88.h |   28 ---
 4 files changed, 64 insertions(+), 20 deletions(-)

diff --git a/drivers/media/video/cx88/cx88-core.c 
b/drivers/media/video/cx88/cx88-core.c
index 8b21457..b0cf307 100644
--- a/drivers/media/video/cx88/cx88-core.c
+++ b/drivers/media/video/cx88/cx88-core.c
@@ -879,7 +879,7 @@ static int set_tvaudio(struct cx88_core *core)
} else {
printk(%s/0: tvaudio support needs work for this tv norm [%s], 
sorry\n,
   core-name, v4l2_norm_to_name(core-tvnorm));
-   core-tvaudio = 0;
+   core-tvaudio = WW_NONE;
return 0;
}
 
diff --git a/drivers/media/video/cx88/cx88-dsp.c 
b/drivers/media/video/cx88/cx88-dsp.c
index a94e00a..e1d6eef 100644
--- a/drivers/media/video/cx88/cx88-dsp.c
+++ b/drivers/media/video/cx88/cx88-dsp.c
@@ -175,7 +175,13 @@ static s32 detect_a2_a2m_eiaj(struct cx88_core *core, s16 
x[], u32 N)
stereo_freq = FREQ_EIAJ_STEREO;
dual_freq = FREQ_EIAJ_DUAL;
break;
-   default:
+   case WW_NONE:
+   case WW_BTSC:
+   case WW_I:
+   case WW_L:
+   case WW_I2SPT:
+   case WW_FM:
+   case WW_I2SADC:
printk(KERN_WARNING %s/0: unsupported audio mode %d for %s\n,
   core-name, core-tvaudio, __func__);
return UNSET;
@@ -292,11 +298,20 @@ s32 cx88_dsp_detect_stereo_sap(struct cx88_core *core)
switch (core-tvaudio) {
case WW_BG:
case WW_DK:
+   case WW_EIAJ:
+   case WW_M:
ret = detect_a2_a2m_eiaj(core, samples, N);
break;
case WW_BTSC:
ret = detect_btsc(core, samples, N);
break;
+   case WW_NONE:
+   case WW_I:
+   case WW_L:
+   case WW_I2SPT:
+   case WW_FM:
+   case WW_I2SADC:
+   break;
}
 
kfree(samples);
diff --git a/drivers/media/video/cx88/cx88-tvaudio.c 
b/drivers/media/video/cx88/cx88-tvaudio.c
index 2396315..db63547 100644
--- a/drivers/media/video/cx88/cx88-tvaudio.c
+++ b/drivers/media/video/cx88/cx88-tvaudio.c
@@ -360,7 +360,15 @@ static void set_audio_standard_NICAM(struct cx88_core 
*core, u32 mode)
set_audio_registers(core, nicam_bgdki_common);
set_audio_registers(core, nicam_i);
break;
-   default:
+   case WW_NONE:
+   case WW_BTSC:
+   case WW_BG:
+   case WW_DK:
+   case WW_EIAJ:
+   case WW_I2SPT:
+   case WW_FM:
+   case WW_I2SADC:
+   case WW_M:
dprintk(%s PAL-BGDK NICAM (status: known-good)\n, __func__);
set_audio_registers(core, nicam_bgdki_common);
set_audio_registers(core, nicam_default);
@@ -621,7 +629,13 @@ static void set_audio_standard_A2(struct cx88_core *core, 
u32 mode)
dprintk(%s AM-L (status: devel)\n, __func__);
set_audio_registers(core, am_l);
break;
-   default:
+   case WW_NONE:
+   case WW_BTSC:
+   case WW_EIAJ:
+   case WW_I2SPT:
+   case WW_FM:
+   case WW_I2SADC:
+   case WW_M:
dprintk(%s Warning: wrong value\n, __func__);
return;
break;
@@ -779,7 +793,7 @@ void cx88_set_tvaudio(struct cx88_core *core)
set_audio_finish(core, EN_I2SIN_ENABLE);
break;
case WW_NONE:
-   default:
+   case WW_I2SPT:
printk(%s/0: unknown tv audio mode [%d]\n,
   core-name, core-tvaudio);
break;
@@ -840,7 +854,12 @@ void cx88_get_stereo(struct cx88_core *core, struct 
v4l2_tuner *t)
break;
}
break;
-   default:
+   case WW_NONE:
+   case WW_I:
+   case WW_L:
+   case WW_I2SPT:
+   case WW_FM:
+   case WW_I2SADC:
/* nothing */
break;
}
@@ -945,6 +964,9 @@ void cx88_set_stereo(struct cx88_core *core, u32 mode, int 
manual)
}
break;
case WW_I2SADC:
+   case WW_NONE:
+   case WW_EIAJ:
+   case WW_I2SPT:
/* DO NOTHING */
break;
}
@@ -1000,7 +1022,12 @@ int cx88_audio_thread(void *data)
/* automatically switch to