* [PATCH] em28xx: fix I2S audio sample rate definitions and info output
@ 2013-12-22 14:17 Frank Schäfer
2013-12-22 16:09 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 3+ messages in thread
From: Frank Schäfer @ 2013-12-22 14:17 UTC (permalink / raw)
To: m.chehab; +Cc: linux-media, Frank Schäfer
The audio configuration in chip config register 0x00 and eeprom are always
consistent. But currently the audio configuration #defines for the chip config
register say 0x20 means 3 sample rates and 0x30 5 sample rates, while the eeprom
info output says 0x20 means 1 sample rate and 0x30 3 sample rates.
I've checked the datasheet excerpts I have and it seems that the meaning of
these bits is different for em2820/40 (1 and 3 sample rates) and em2860+
(3 and 5 smaple rates).
I have also checked my Hauppauge WinTV USB 2 (em2840) and the chip/eeprom
audio config 0x20 matches the sample rates reproted by the USB device
descriptor (32k only).
Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
---
drivers/media/usb/em28xx/em28xx-core.c | 24 +++++++++++++-----------
drivers/media/usb/em28xx/em28xx-i2c.c | 10 ++++++++--
drivers/media/usb/em28xx/em28xx-reg.h | 10 ++++++----
drivers/media/usb/em28xx/em28xx.h | 3 +--
4 Dateien geändert, 28 Zeilen hinzugefügt(+), 19 Zeilen entfernt(-)
diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
index f6076a5..192b657 100644
--- a/drivers/media/usb/em28xx/em28xx-core.c
+++ b/drivers/media/usb/em28xx/em28xx-core.c
@@ -525,17 +525,19 @@ int em28xx_audio_setup(struct em28xx *dev)
dev->has_alsa_audio = false;
dev->audio_mode.has_audio = false;
return 0;
- } else if ((cfg & EM28XX_CHIPCFG_AUDIOMASK) ==
- EM28XX_CHIPCFG_I2S_3_SAMPRATES) {
- em28xx_info("I2S Audio (3 sample rates)\n");
- dev->audio_mode.i2s_3rates = 1;
- } else if ((cfg & EM28XX_CHIPCFG_AUDIOMASK) ==
- EM28XX_CHIPCFG_I2S_5_SAMPRATES) {
- em28xx_info("I2S Audio (5 sample rates)\n");
- dev->audio_mode.i2s_5rates = 1;
- }
-
- if ((cfg & EM28XX_CHIPCFG_AUDIOMASK) != EM28XX_CHIPCFG_AC97) {
+ } else if ((cfg & EM28XX_CHIPCFG_AUDIOMASK) != EM28XX_CHIPCFG_AC97) {
+ if (dev->chip_id < CHIP_ID_EM2860 &&
+ (cfg & EM28XX_CHIPCFG_AUDIOMASK) ==
+ EM2820_CHIPCFG_I2S_1_SAMPRATE)
+ dev->audio_mode.i2s_samplerates = 1;
+ else if (dev->chip_id >= CHIP_ID_EM2860 &&
+ (cfg & EM28XX_CHIPCFG_AUDIOMASK) ==
+ EM2860_CHIPCFG_I2S_5_SAMPRATES)
+ dev->audio_mode.i2s_samplerates = 5;
+ else
+ dev->audio_mode.i2s_samplerates = 3;
+ em28xx_info("I2S Audio (%d sample rate(s))\n",
+ dev->audio_mode.i2s_samplerates);
/* Skip the code that does AC97 vendor detection */
dev->audio_mode.ac97 = EM28XX_NO_AC97;
goto init_audio;
diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
index c4ff973..f2d5f8a 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -736,10 +736,16 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned bus,
em28xx_info("\tAC97 audio (5 sample rates)\n");
break;
case 2:
- em28xx_info("\tI2S audio, sample rate=32k\n");
+ if (dev->chip_id < CHIP_ID_EM2860)
+ em28xx_info("\tI2S audio, sample rate=32k\n");
+ else
+ em28xx_info("\tI2S audio, 3 sample rates\n");
break;
case 3:
- em28xx_info("\tI2S audio, 3 sample rates\n");
+ if (dev->chip_id < CHIP_ID_EM2860)
+ em28xx_info("\tI2S audio, 3 sample rates\n");
+ else
+ em28xx_info("\tI2S audio, 5 sample rates\n");
break;
}
diff --git a/drivers/media/usb/em28xx/em28xx-reg.h b/drivers/media/usb/em28xx/em28xx-reg.h
index b769ceb..311fb34 100644
--- a/drivers/media/usb/em28xx/em28xx-reg.h
+++ b/drivers/media/usb/em28xx/em28xx-reg.h
@@ -25,10 +25,12 @@
#define EM28XX_R00_CHIPCFG 0x00
/* em28xx Chip Configuration 0x00 */
-#define EM28XX_CHIPCFG_VENDOR_AUDIO 0x80
-#define EM28XX_CHIPCFG_I2S_VOLUME_CAPABLE 0x40
-#define EM28XX_CHIPCFG_I2S_5_SAMPRATES 0x30
-#define EM28XX_CHIPCFG_I2S_3_SAMPRATES 0x20
+#define EM2860_CHIPCFG_VENDOR_AUDIO 0x80
+#define EM2860_CHIPCFG_I2S_VOLUME_CAPABLE 0x40
+#define EM2820_CHIPCFG_I2S_3_SAMPRATES 0x30
+#define EM2860_CHIPCFG_I2S_5_SAMPRATES 0x30
+#define EM2820_CHIPCFG_I2S_1_SAMPRATE 0x20
+#define EM2860_CHIPCFG_I2S_3_SAMPRATES 0x20
#define EM28XX_CHIPCFG_AC97 0x10
#define EM28XX_CHIPCFG_AUDIOMASK 0x30
diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
index 191ef35..4d8c7d2 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -292,8 +292,7 @@ struct em28xx_audio_mode {
unsigned int has_audio:1;
- unsigned int i2s_3rates:1;
- unsigned int i2s_5rates:1;
+ u8 i2s_samplerates;
};
/* em28xx has two audio inputs: tuner and line in.
--
1.7.10.4
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] em28xx: fix I2S audio sample rate definitions and info output
2013-12-22 14:17 [PATCH] em28xx: fix I2S audio sample rate definitions and info output Frank Schäfer
@ 2013-12-22 16:09 ` Mauro Carvalho Chehab
2013-12-22 18:15 ` Frank Schäfer
0 siblings, 1 reply; 3+ messages in thread
From: Mauro Carvalho Chehab @ 2013-12-22 16:09 UTC (permalink / raw)
To: Frank Schäfer; +Cc: linux-media
Em Sun, 22 Dec 2013 15:17:46 +0100
Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
> The audio configuration in chip config register 0x00 and eeprom are always
> consistent. But currently the audio configuration #defines for the chip config
> register say 0x20 means 3 sample rates and 0x30 5 sample rates, while the eeprom
> info output says 0x20 means 1 sample rate and 0x30 3 sample rates.
>
> I've checked the datasheet excerpts I have and it seems that the meaning of
> these bits is different for em2820/40 (1 and 3 sample rates) and em2860+
> (3 and 5 smaple rates).
> I have also checked my Hauppauge WinTV USB 2 (em2840) and the chip/eeprom
> audio config 0x20 matches the sample rates reproted by the USB device
> descriptor (32k only).
>
> Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
> ---
> drivers/media/usb/em28xx/em28xx-core.c | 24 +++++++++++++-----------
> drivers/media/usb/em28xx/em28xx-i2c.c | 10 ++++++++--
> drivers/media/usb/em28xx/em28xx-reg.h | 10 ++++++----
> drivers/media/usb/em28xx/em28xx.h | 3 +--
> 4 Dateien geändert, 28 Zeilen hinzugefügt(+), 19 Zeilen entfernt(-)
>
> diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
> index f6076a5..192b657 100644
> --- a/drivers/media/usb/em28xx/em28xx-core.c
> +++ b/drivers/media/usb/em28xx/em28xx-core.c
> @@ -525,17 +525,19 @@ int em28xx_audio_setup(struct em28xx *dev)
> dev->has_alsa_audio = false;
> dev->audio_mode.has_audio = false;
> return 0;
> - } else if ((cfg & EM28XX_CHIPCFG_AUDIOMASK) ==
> - EM28XX_CHIPCFG_I2S_3_SAMPRATES) {
> - em28xx_info("I2S Audio (3 sample rates)\n");
> - dev->audio_mode.i2s_3rates = 1;
> - } else if ((cfg & EM28XX_CHIPCFG_AUDIOMASK) ==
> - EM28XX_CHIPCFG_I2S_5_SAMPRATES) {
> - em28xx_info("I2S Audio (5 sample rates)\n");
> - dev->audio_mode.i2s_5rates = 1;
> - }
> -
> - if ((cfg & EM28XX_CHIPCFG_AUDIOMASK) != EM28XX_CHIPCFG_AC97) {
> + } else if ((cfg & EM28XX_CHIPCFG_AUDIOMASK) != EM28XX_CHIPCFG_AC97) {
> + if (dev->chip_id < CHIP_ID_EM2860 &&
> + (cfg & EM28XX_CHIPCFG_AUDIOMASK) ==
> + EM2820_CHIPCFG_I2S_1_SAMPRATE)
> + dev->audio_mode.i2s_samplerates = 1;
No need to store it at all, as, at least currently, this is not used
anywhere.
This patch could be useful if we could use this to improve em28xx-audio.c.
Otherwise, I would just drop the code that checks it, eventually
just keeping the registers at em28xx-reg.h
> + else if (dev->chip_id >= CHIP_ID_EM2860 &&
> + (cfg & EM28XX_CHIPCFG_AUDIOMASK) ==
> + EM2860_CHIPCFG_I2S_5_SAMPRATES)
> + dev->audio_mode.i2s_samplerates = 5;
> + else
> + dev->audio_mode.i2s_samplerates = 3;
> + em28xx_info("I2S Audio (%d sample rate(s))\n",
> + dev->audio_mode.i2s_samplerates);
> /* Skip the code that does AC97 vendor detection */
> dev->audio_mode.ac97 = EM28XX_NO_AC97;
> goto init_audio;
> diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
> index c4ff973..f2d5f8a 100644
> --- a/drivers/media/usb/em28xx/em28xx-i2c.c
> +++ b/drivers/media/usb/em28xx/em28xx-i2c.c
> @@ -736,10 +736,16 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned bus,
> em28xx_info("\tAC97 audio (5 sample rates)\n");
> break;
> case 2:
> - em28xx_info("\tI2S audio, sample rate=32k\n");
> + if (dev->chip_id < CHIP_ID_EM2860)
> + em28xx_info("\tI2S audio, sample rate=32k\n");
> + else
> + em28xx_info("\tI2S audio, 3 sample rates\n");
> break;
> case 3:
> - em28xx_info("\tI2S audio, 3 sample rates\n");
> + if (dev->chip_id < CHIP_ID_EM2860)
> + em28xx_info("\tI2S audio, 3 sample rates\n");
> + else
> + em28xx_info("\tI2S audio, 5 sample rates\n");
> break;
> }
>
> diff --git a/drivers/media/usb/em28xx/em28xx-reg.h b/drivers/media/usb/em28xx/em28xx-reg.h
> index b769ceb..311fb34 100644
> --- a/drivers/media/usb/em28xx/em28xx-reg.h
> +++ b/drivers/media/usb/em28xx/em28xx-reg.h
> @@ -25,10 +25,12 @@
> #define EM28XX_R00_CHIPCFG 0x00
>
> /* em28xx Chip Configuration 0x00 */
> -#define EM28XX_CHIPCFG_VENDOR_AUDIO 0x80
> -#define EM28XX_CHIPCFG_I2S_VOLUME_CAPABLE 0x40
> -#define EM28XX_CHIPCFG_I2S_5_SAMPRATES 0x30
> -#define EM28XX_CHIPCFG_I2S_3_SAMPRATES 0x20
> +#define EM2860_CHIPCFG_VENDOR_AUDIO 0x80
> +#define EM2860_CHIPCFG_I2S_VOLUME_CAPABLE 0x40
> +#define EM2820_CHIPCFG_I2S_3_SAMPRATES 0x30
> +#define EM2860_CHIPCFG_I2S_5_SAMPRATES 0x30
> +#define EM2820_CHIPCFG_I2S_1_SAMPRATE 0x20
> +#define EM2860_CHIPCFG_I2S_3_SAMPRATES 0x20
> #define EM28XX_CHIPCFG_AC97 0x10
> #define EM28XX_CHIPCFG_AUDIOMASK 0x30
>
> diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
> index 191ef35..4d8c7d2 100644
> --- a/drivers/media/usb/em28xx/em28xx.h
> +++ b/drivers/media/usb/em28xx/em28xx.h
> @@ -292,8 +292,7 @@ struct em28xx_audio_mode {
>
> unsigned int has_audio:1;
>
> - unsigned int i2s_3rates:1;
> - unsigned int i2s_5rates:1;
> + u8 i2s_samplerates;
> };
>
> /* em28xx has two audio inputs: tuner and line in.
--
Cheers,
Mauro
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] em28xx: fix I2S audio sample rate definitions and info output
2013-12-22 16:09 ` Mauro Carvalho Chehab
@ 2013-12-22 18:15 ` Frank Schäfer
0 siblings, 0 replies; 3+ messages in thread
From: Frank Schäfer @ 2013-12-22 18:15 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: Linux Media Mailing List
Am 22.12.2013 17:09, schrieb Mauro Carvalho Chehab:
> Em Sun, 22 Dec 2013 15:17:46 +0100
> Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
>
>> The audio configuration in chip config register 0x00 and eeprom are always
>> consistent. But currently the audio configuration #defines for the chip config
>> register say 0x20 means 3 sample rates and 0x30 5 sample rates, while the eeprom
>> info output says 0x20 means 1 sample rate and 0x30 3 sample rates.
>>
>> I've checked the datasheet excerpts I have and it seems that the meaning of
>> these bits is different for em2820/40 (1 and 3 sample rates) and em2860+
>> (3 and 5 smaple rates).
>> I have also checked my Hauppauge WinTV USB 2 (em2840) and the chip/eeprom
>> audio config 0x20 matches the sample rates reproted by the USB device
>> descriptor (32k only).
>>
>> Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
>> ---
>> drivers/media/usb/em28xx/em28xx-core.c | 24 +++++++++++++-----------
>> drivers/media/usb/em28xx/em28xx-i2c.c | 10 ++++++++--
>> drivers/media/usb/em28xx/em28xx-reg.h | 10 ++++++----
>> drivers/media/usb/em28xx/em28xx.h | 3 +--
>> 4 Dateien geändert, 28 Zeilen hinzugefügt(+), 19 Zeilen entfernt(-)
>>
>> diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
>> index f6076a5..192b657 100644
>> --- a/drivers/media/usb/em28xx/em28xx-core.c
>> +++ b/drivers/media/usb/em28xx/em28xx-core.c
>> @@ -525,17 +525,19 @@ int em28xx_audio_setup(struct em28xx *dev)
>> dev->has_alsa_audio = false;
>> dev->audio_mode.has_audio = false;
>> return 0;
>> - } else if ((cfg & EM28XX_CHIPCFG_AUDIOMASK) ==
>> - EM28XX_CHIPCFG_I2S_3_SAMPRATES) {
>> - em28xx_info("I2S Audio (3 sample rates)\n");
>> - dev->audio_mode.i2s_3rates = 1;
>> - } else if ((cfg & EM28XX_CHIPCFG_AUDIOMASK) ==
>> - EM28XX_CHIPCFG_I2S_5_SAMPRATES) {
>> - em28xx_info("I2S Audio (5 sample rates)\n");
>> - dev->audio_mode.i2s_5rates = 1;
>> - }
>> -
>> - if ((cfg & EM28XX_CHIPCFG_AUDIOMASK) != EM28XX_CHIPCFG_AC97) {
>> + } else if ((cfg & EM28XX_CHIPCFG_AUDIOMASK) != EM28XX_CHIPCFG_AC97) {
>> + if (dev->chip_id < CHIP_ID_EM2860 &&
>> + (cfg & EM28XX_CHIPCFG_AUDIOMASK) ==
>> + EM2820_CHIPCFG_I2S_1_SAMPRATE)
>> + dev->audio_mode.i2s_samplerates = 1;
> No need to store it at all, as, at least currently, this is not used
> anywhere.
Yes, and the same applies to most other members of struct
em28xx_audio_mode. ;)
I'm currently looking deep into the audio code and the whole thing seems
to be hopelessly messed up.
The plan is to send a separate patch that cleans up the audio structures
and variables later when I'm sure which way to go to fix things.
Would that be ok for you or do you want me to drop the samplerate fields
now ?
Regards,
Frank
> This patch could be useful if we could use this to improve em28xx-audio.c.
>
> Otherwise, I would just drop the code that checks it, eventually
> just keeping the registers at em28xx-reg.h
>
>> + else if (dev->chip_id >= CHIP_ID_EM2860 &&
>> + (cfg & EM28XX_CHIPCFG_AUDIOMASK) ==
>> + EM2860_CHIPCFG_I2S_5_SAMPRATES)
>> + dev->audio_mode.i2s_samplerates = 5;
>> + else
>> + dev->audio_mode.i2s_samplerates = 3;
>> + em28xx_info("I2S Audio (%d sample rate(s))\n",
>> + dev->audio_mode.i2s_samplerates);
>> /* Skip the code that does AC97 vendor detection */
>> dev->audio_mode.ac97 = EM28XX_NO_AC97;
>> goto init_audio;
>> diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
>> index c4ff973..f2d5f8a 100644
>> --- a/drivers/media/usb/em28xx/em28xx-i2c.c
>> +++ b/drivers/media/usb/em28xx/em28xx-i2c.c
>> @@ -736,10 +736,16 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned bus,
>> em28xx_info("\tAC97 audio (5 sample rates)\n");
>> break;
>> case 2:
>> - em28xx_info("\tI2S audio, sample rate=32k\n");
>> + if (dev->chip_id < CHIP_ID_EM2860)
>> + em28xx_info("\tI2S audio, sample rate=32k\n");
>> + else
>> + em28xx_info("\tI2S audio, 3 sample rates\n");
>> break;
>> case 3:
>> - em28xx_info("\tI2S audio, 3 sample rates\n");
>> + if (dev->chip_id < CHIP_ID_EM2860)
>> + em28xx_info("\tI2S audio, 3 sample rates\n");
>> + else
>> + em28xx_info("\tI2S audio, 5 sample rates\n");
>> break;
>> }
>>
>> diff --git a/drivers/media/usb/em28xx/em28xx-reg.h b/drivers/media/usb/em28xx/em28xx-reg.h
>> index b769ceb..311fb34 100644
>> --- a/drivers/media/usb/em28xx/em28xx-reg.h
>> +++ b/drivers/media/usb/em28xx/em28xx-reg.h
>> @@ -25,10 +25,12 @@
>> #define EM28XX_R00_CHIPCFG 0x00
>>
>> /* em28xx Chip Configuration 0x00 */
>> -#define EM28XX_CHIPCFG_VENDOR_AUDIO 0x80
>> -#define EM28XX_CHIPCFG_I2S_VOLUME_CAPABLE 0x40
>> -#define EM28XX_CHIPCFG_I2S_5_SAMPRATES 0x30
>> -#define EM28XX_CHIPCFG_I2S_3_SAMPRATES 0x20
>> +#define EM2860_CHIPCFG_VENDOR_AUDIO 0x80
>> +#define EM2860_CHIPCFG_I2S_VOLUME_CAPABLE 0x40
>> +#define EM2820_CHIPCFG_I2S_3_SAMPRATES 0x30
>> +#define EM2860_CHIPCFG_I2S_5_SAMPRATES 0x30
>> +#define EM2820_CHIPCFG_I2S_1_SAMPRATE 0x20
>> +#define EM2860_CHIPCFG_I2S_3_SAMPRATES 0x20
>> #define EM28XX_CHIPCFG_AC97 0x10
>> #define EM28XX_CHIPCFG_AUDIOMASK 0x30
>>
>> diff --git a/drivers/media/usb/em28xx/em28xx.h b/drivers/media/usb/em28xx/em28xx.h
>> index 191ef35..4d8c7d2 100644
>> --- a/drivers/media/usb/em28xx/em28xx.h
>> +++ b/drivers/media/usb/em28xx/em28xx.h
>> @@ -292,8 +292,7 @@ struct em28xx_audio_mode {
>>
>> unsigned int has_audio:1;
>>
>> - unsigned int i2s_3rates:1;
>> - unsigned int i2s_5rates:1;
>> + u8 i2s_samplerates;
>> };
>>
>> /* em28xx has two audio inputs: tuner and line in.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-12-22 18:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-22 14:17 [PATCH] em28xx: fix I2S audio sample rate definitions and info output Frank Schäfer
2013-12-22 16:09 ` Mauro Carvalho Chehab
2013-12-22 18:15 ` Frank Schäfer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox