At Fri, 15 May 2009 02:34:07 +0200, Torben Schulz wrote:
With this patch the ALC889A chip will be identified correctly. The functions patch_alc882() and patch_alc883() have been altered to set the stream names correctly. Without this patch 'aplay -l' would identify an ALC889A chip as ALC885 for example. Apart from that a kernel info message will be printed if ALC885 or ALC889A chips are handled by the wrong patch function.
Signed-off by: Torben Schulz public@letorbi.de
Thanks for the patch! Just a few comments below...
--- alsa-driver/alsa-kernel/pci/hda/patch_realtek.c 2009-05-14 18:23:17.000000000 +0200 +++ alsa-driver-mod/alsa-kernel/pci/hda/patch_realtek.c 2009-05-15 01:08:16.000000000 +0200 @@ -7338,8 +7338,17 @@ setup_preset(spec, &alc882_presets[board_config]);
if (codec->vendor_id == 0x10ec0885) {
spec->stream_name_analog = "ALC885 Analog";spec->stream_name_digital = "ALC885 Digital";
if ((codec->revision_id == 0x100101) ||(codec->revision_id == 0x100103)) {spec->stream_name_analog = "ALC889A Analog";spec->stream_name_digital = "ALC889A Digital";printk(KERN_INFO"hda_codec: ALC889A handled by patch_alc882(),"" should be handled by patch_alc883()\n");
It's rather a driver internal thing and not necessarily to be told to each user :) If any, let it be a verbose debug print with snd_printdd().
} else {spec->stream_name_analog = "ALC885 Analog";spec->stream_name_digital = "ALC885 Digital";}@@ -9231,6 +9240,27 @@ setup_preset(spec, &alc883_presets[board_config]);
switch (codec->vendor_id) {
- case 0x10ec0885:
if ((codec->revision_id == 0x100101) ||(codec->revision_id == 0x100103)) {
This if block can be skipped (thus less indent level) by ...
spec->stream_name_analog = "ALC889A Analog";spec->stream_name_digital = "ALC889A Digital";if (!spec->num_adc_nids) {spec->num_adc_nids = ARRAY_SIZE(alc883_adc_nids);spec->adc_nids = alc883_adc_nids;}if (!spec->capsrc_nids)spec->capsrc_nids = alc883_capsrc_nids;spec->capture_style = CAPT_MIX; /* matrix-style capture */spec->init_amp = ALC_INIT_DEFAULT; /* always initialize */} else {spec->stream_name_analog = "ALC885 Analog";spec->stream_name_digital = "ALC885 Digital";printk(KERN_INFO"hda_codec: ALC885 handled by patch_alc883(),"" should be handled by patch_alc882()\n");
... checking the ALC889A at the beginning of patch_alc883() since using patch_alc883() for ALC885 is an error indeed.
Could you fix and repost?
BTW, regarding the strings: basically we can do a better clean-up by using already existing strings. For example, make new fields, hda_codec.vendor_name and hda_codec.chip_name, which points the vendor id string in hda_codec.c and the chip name string in snd_hda_codec_preset struct respectively. Then all PCM name strings can be created automatically.
But, it's another level of clean-up to be done later, after your fixes.
thanks,
Takashi