Thanks for the comments and I will refine the patch. > -----Original Message----- > From: Takashi Iwai [mailto:tiwai@xxxxxxx] > Sent: Tuesday, November 11, 2008 4:17 PM > To: Yang, Libin > Cc: alsa-devel@xxxxxxxxxxxxxxxx > Subject: Re: > > At Tue, 11 Nov 2008 10:43:12 +0800, > Yang, Libin wrote: > > > > Hi Takashi, > > > > This patch is to support detecting new AMD HD Audio devices with HDA PCI > > class code. Would you please review it? Thanks a lot! > > Thanks for the patch! Some quick review comments below... > > > > --- alsa-driver-1.0.18.orig/alsa-kernel/pci/hda/hda_intel.c > > 2008-10-29 20:41:35.000000000 +0800 > > +++ alsa-driver-1.0.18/alsa-kernel/pci/hda/hda_intel.c 2008-11-11 > > 18:33:14.000000000 +0800 > > @@ -291,6 +291,7 @@ > > /* Define VIA HD Audio Device ID*/ > > #define VIA_HDAC_DEVICE_ID 0x3288 > > > > +#define PCI_CLASS_MULTIMEDIA_HDA 0x040300 > > Put this into include/linux/pci_ids.h as a common definition. > And, IMO, it'd be better to name it *_HD_AUDIO than *_HDA. > > > > > /* > > */ > > @@ -410,6 +411,7 @@ > > AZX_DRIVER_ULI, > > AZX_DRIVER_NVIDIA, > > AZX_DRIVER_TERA, > > + AZX_DRIVER_AMD_AUTO, > > We can create a more generic entry, say, AZX_DRIVER_GENERIC. > > > > @@ -423,6 +425,7 @@ > > [AZX_DRIVER_ULI] = "HDA ULI M5461", > > [AZX_DRIVER_NVIDIA] = "HDA NVidia", > > [AZX_DRIVER_TERA] = "HDA Teradici", > > + [AZX_DRIVER_AMD_AUTO] = "HDA AMD", > > And, make it [AZX_DRIVER_GENERIC] = "HD-Audio Generic" or so. > > > > static unsigned int azx_default_codecs[AZX_NUM_DRIVERS] __devinitdata = > > { > > [AZX_DRIVER_ICH] = 3, > > [AZX_DRIVER_ATI] = 3, > > + [AZX_DRIVER_AMD_AUTO] = 3, > > I thought you'll have up to 4 codecs? > > > > static int __devinit azx_codec_create(struct azx *chip, const char > > *model, > > @@ -2146,6 +2150,7 @@ > > chip->playback_streams = ULI_NUM_PLAYBACK; > > chip->capture_streams = ULI_NUM_CAPTURE; > > break; > > + case AZX_DRIVER_AMD_AUTO: > > AZX_DRIVER_GENERIC can go to "default". > > > case AZX_DRIVER_ATIHDMI: > > chip->playback_streams = ATIHDMI_NUM_PLAYBACK; > > chip->capture_streams = ATIHDMI_NUM_CAPTURE; > > @@ -2373,6 +2378,9 @@ > > { PCI_DEVICE(0x10de, 0x0bd7), .driver_data = AZX_DRIVER_NVIDIA > > }, > > /* Teradici */ > > { PCI_DEVICE(0x6549, 0x1200), .driver_data = AZX_DRIVER_TERA }, > > + /* AMD Generic, PCI class code and Vendor ID for HD Audio */ > > + { PCI_VENDOR_ID_ATI, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID, > > + PCI_CLASS_MULTIMEDIA_HDA, 0xffffff, AZX_DRIVER_AMD_AUTO }, > > Use PCI_DEVICE() macro and C99 style initialization, such as, > > { PCI_DEVICE(PCI_VENDOR_ID_ATI, PCI_ANY_ID), > .class = PCI_CLASS_MULTIMEDIA_HD_AUDIO, > .class_mask = 0xffffff, > .driver_data = AZX_DRIVER_GENERIC }, > > thanks, > > Takashi _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
