Re: [PATCH 4/4] em28xx: get rid of structs em28xx_ac97_mode and em28xx_audio_mode
Am 23.09.2014 um 02:02 schrieb Mauro Carvalho Chehab:
Em Sat, 13 Sep 2014 10:52:22 +0200
Frank Schäfer fschaefer@googlemail.com escreveu:
Now that we have enum em28xx_int_audio (none/i2s/ac97), it is no longer
necessary to check dev-audio_mode.ac97 to determine the type of internal
audio connection.
There is also no need to save the type of the detected AC97 chip.
Removing the AC97 chip is a bad idea, as the mux of each AC97 device
is different.
I don't remember anymore what device comes with the sigmatel chips,
but it does have a different mixer than em202. The logic to set it
different is not there basically for two reasons:
1) I don't have the device with sigmatel (I think someone borrowed it to me
at that time, and for a very limited period of time);
2) there's a hole ac97 support at /sound. The idea is to re-use it instead
of reinventing the wheel, but that requires time and a few different AC97
setups.
1.) WHICH devices to you think need AC97 chip type specific code ?
2.) WHEN are you going to implement it ?
It's just a big dead ugly chunk of code, which causes confusion.
So please, let's simplify the code.
If we really need to distinguish between AC97 chips one day, we can
reintroduce it again.
Please note that I'm not removing the AC97 chip type detection, so we
are not loosing any information that might be useful in the future.
Btw, there are other em28xx devices with other non-em202 AC97 chips out
there, with different settings (and even different sampling rates).
Those AC97 devices are generally found at the grabber devices.
I know. The VAD Laplace webcam is one of them. And the current audio
code is broken for it.
But the reason for that isn't that we do not distinguish enough between
AC97 chips.
The reason for that is, that we touch AC97 _at_all_ for USB audio class
devices, which is evil, unnecessary and therefore also not done by the
Windows driver.
The situation is of course different for devices with usb vendor class
audio (em28xx-alsa).
But that's a separate issue which has nothing to do with this patch... ;-)
Regards,
Frank
Regards,
Mauro
So replce the remaining checks of dev-audio_mode.ac97 with equivalent checks
of dev-int_audio_type and get rid of struct em28xx_ac97_mode and finally the
whole struct em28xx_audio_mode.
Signed-off-by: Frank Schäfer fschaefer@googlemail.com
---
drivers/media/usb/em28xx/em28xx-audio.c | 2 +-
drivers/media/usb/em28xx/em28xx-core.c | 36
++---
drivers/media/usb/em28xx/em28xx-video.c | 2 +-
drivers/media/usb/em28xx/em28xx.h | 13
4 files changed, 8 insertions(+), 45 deletions(-)
diff --git a/drivers/media/usb/em28xx/em28xx-audio.c
b/drivers/media/usb/em28xx/em28xx-audio.c
index 90c7a83..c3a4224 100644
--- a/drivers/media/usb/em28xx/em28xx-audio.c
+++ b/drivers/media/usb/em28xx/em28xx-audio.c
@@ -933,7 +933,7 @@ static int em28xx_audio_init(struct em28xx *dev)
INIT_WORK(adev-wq_trigger, audio_trigger);
-if (dev-audio_mode.ac97 != EM28XX_NO_AC97) {
+if (dev-int_audio_type == EM28XX_INT_AUDIO_AC97) {
em28xx_cvol_new(card, dev, Video, AC97_VIDEO);
em28xx_cvol_new(card, dev, Line In, AC97_LINE);
em28xx_cvol_new(card, dev, Phone, AC97_PHONE);
diff --git a/drivers/media/usb/em28xx/em28xx-core.c
b/drivers/media/usb/em28xx/em28xx-core.c
index ed83e4e..7464e70 100644
--- a/drivers/media/usb/em28xx/em28xx-core.c
+++ b/drivers/media/usb/em28xx/em28xx-core.c
@@ -405,12 +405,8 @@ static int em28xx_set_audio_source(struct em28xx *dev)
return ret;
msleep(5);
-switch (dev-audio_mode.ac97) {
-case EM28XX_NO_AC97:
-break;
-default:
+if (dev-int_audio_type == EM28XX_INT_AUDIO_AC97)
ret = set_ac97_input(dev);
-}
return ret;
}
@@ -439,7 +435,7 @@ int em28xx_audio_analog_set(struct em28xx *dev)
/* It is assumed that all devices use master volume for output.
It would be possible to use also line output.
*/
-if (dev-audio_mode.ac97 != EM28XX_NO_AC97) {
+if (dev-int_audio_type == EM28XX_INT_AUDIO_AC97) {
/* Mute all outputs */
for (i = 0; i ARRAY_SIZE(outputs); i++) {
ret = em28xx_write_ac97(dev, outputs[i].reg, 0x8000);
@@ -462,7 +458,7 @@ int em28xx_audio_analog_set(struct em28xx *dev)
ret = em28xx_set_audio_source(dev);
/* Sets volume */
-if (dev-audio_mode.ac97 != EM28XX_NO_AC97) {
+if (dev-int_audio_type == EM28XX_INT_AUDIO_AC97) {
int vol;
em28xx_write_ac97(dev, AC97_POWERDOWN, 0x4200);
@@ -544,14 +540,11 @@ int em28xx_audio_setup(struct em28xx *dev)
em28xx_info(I2S Audio (%d sample rate(s))\n,
i2s_samplerates);
/* Skip the code that does AC97 vendor detection */
-dev-audio_mode.ac97 =
Re: [PATCH 4/4] em28xx: get rid of structs em28xx_ac97_mode and em28xx_audio_mode
Em Sat, 13 Sep 2014 10:52:22 +0200
Frank Schäfer fschaefer@googlemail.com escreveu:
Now that we have enum em28xx_int_audio (none/i2s/ac97), it is no longer
necessary to check dev-audio_mode.ac97 to determine the type of internal
audio connection.
There is also no need to save the type of the detected AC97 chip.
Removing the AC97 chip is a bad idea, as the mux of each AC97 device
is different.
I don't remember anymore what device comes with the sigmatel chips,
but it does have a different mixer than em202. The logic to set it
different is not there basically for two reasons:
1) I don't have the device with sigmatel (I think someone borrowed it to me
at that time, and for a very limited period of time);
2) there's a hole ac97 support at /sound. The idea is to re-use it instead
of reinventing the wheel, but that requires time and a few different AC97
setups.
Btw, there are other em28xx devices with other non-em202 AC97 chips out
there, with different settings (and even different sampling rates).
Those AC97 devices are generally found at the grabber devices.
Regards,
Mauro
So replce the remaining checks of dev-audio_mode.ac97 with equivalent checks
of dev-int_audio_type and get rid of struct em28xx_ac97_mode and finally the
whole struct em28xx_audio_mode.
Signed-off-by: Frank Schäfer fschaefer@googlemail.com
---
drivers/media/usb/em28xx/em28xx-audio.c | 2 +-
drivers/media/usb/em28xx/em28xx-core.c | 36
++---
drivers/media/usb/em28xx/em28xx-video.c | 2 +-
drivers/media/usb/em28xx/em28xx.h | 13
4 files changed, 8 insertions(+), 45 deletions(-)
diff --git a/drivers/media/usb/em28xx/em28xx-audio.c
b/drivers/media/usb/em28xx/em28xx-audio.c
index 90c7a83..c3a4224 100644
--- a/drivers/media/usb/em28xx/em28xx-audio.c
+++ b/drivers/media/usb/em28xx/em28xx-audio.c
@@ -933,7 +933,7 @@ static int em28xx_audio_init(struct em28xx *dev)
INIT_WORK(adev-wq_trigger, audio_trigger);
- if (dev-audio_mode.ac97 != EM28XX_NO_AC97) {
+ if (dev-int_audio_type == EM28XX_INT_AUDIO_AC97) {
em28xx_cvol_new(card, dev, Video, AC97_VIDEO);
em28xx_cvol_new(card, dev, Line In, AC97_LINE);
em28xx_cvol_new(card, dev, Phone, AC97_PHONE);
diff --git a/drivers/media/usb/em28xx/em28xx-core.c
b/drivers/media/usb/em28xx/em28xx-core.c
index ed83e4e..7464e70 100644
--- a/drivers/media/usb/em28xx/em28xx-core.c
+++ b/drivers/media/usb/em28xx/em28xx-core.c
@@ -405,12 +405,8 @@ static int em28xx_set_audio_source(struct em28xx *dev)
return ret;
msleep(5);
- switch (dev-audio_mode.ac97) {
- case EM28XX_NO_AC97:
- break;
- default:
+ if (dev-int_audio_type == EM28XX_INT_AUDIO_AC97)
ret = set_ac97_input(dev);
- }
return ret;
}
@@ -439,7 +435,7 @@ int em28xx_audio_analog_set(struct em28xx *dev)
/* It is assumed that all devices use master volume for output.
It would be possible to use also line output.
*/
- if (dev-audio_mode.ac97 != EM28XX_NO_AC97) {
+ if (dev-int_audio_type == EM28XX_INT_AUDIO_AC97) {
/* Mute all outputs */
for (i = 0; i ARRAY_SIZE(outputs); i++) {
ret = em28xx_write_ac97(dev, outputs[i].reg, 0x8000);
@@ -462,7 +458,7 @@ int em28xx_audio_analog_set(struct em28xx *dev)
ret = em28xx_set_audio_source(dev);
/* Sets volume */
- if (dev-audio_mode.ac97 != EM28XX_NO_AC97) {
+ if (dev-int_audio_type == EM28XX_INT_AUDIO_AC97) {
int vol;
em28xx_write_ac97(dev, AC97_POWERDOWN, 0x4200);
@@ -544,14 +540,11 @@ int em28xx_audio_setup(struct em28xx *dev)
em28xx_info(I2S Audio (%d sample rate(s))\n,
i2s_samplerates);
/* Skip the code that does AC97 vendor detection */
- dev-audio_mode.ac97 = EM28XX_NO_AC97;
goto init_audio;
} else {
dev-int_audio_type = EM28XX_INT_AUDIO_AC97;
}
- dev-audio_mode.ac97 = EM28XX_AC97_OTHER;
-
vid1 = em28xx_read_ac97(dev, AC97_VENDOR_ID1);
if (vid1 0) {
/*
@@ -560,7 +553,6 @@ int em28xx_audio_setup(struct em28xx *dev)
* CHIPCFG register, even not having an AC97 chip
*/
em28xx_warn(AC97 chip type couldn't be determined\n);
- dev-audio_mode.ac97 = EM28XX_NO_AC97;
if (dev-usb_audio_type == EM28XX_USB_AUDIO_VENDOR)
dev-usb_audio_type = EM28XX_USB_AUDIO_NONE;
dev-int_audio_type = EM28XX_INT_AUDIO_NONE;
@@ -582,30 +574,14 @@ int em28xx_audio_setup(struct em28xx *dev)
/* Try to identify what audio processor we have */
if (((vid == 0x) || (vid == 0x83847650)) (feat == 0x6a90))
- dev-audio_mode.ac97 =
