At Fri, 25 Aug 2006 19:18:08 +0100, James Courtier-Dutton wrote: > > http://alsa-project.org/~james/alsa-driver/emu1212m/emu1212m-10.diff > > This patch added some basic ALSA support for the Creative emu1212m and > emu1820m > > Please review. The patch looks almost good, but just a few minor issues: > +static int snd_emu1212m_load_firmware(struct snd_emu10k1 * emu, const char * filename) > +{ > + int err; > + int n, i; > + int reg; > + int value; > + const struct firmware *fw_entry; > + > + if((err = request_firmware(&fw_entry, filename, &emu->pci->dev)) != 0) { Please fix spaces, not only here but in general around if and for lines. A space if preferred between if and parenthesis, around '=', '<=', etc. > + snd_printk(KERN_INFO "firmware: %s not found. Err=%d\n",filename, err); Better to be KERN_WARNING or KERN_ERR? > + return err; > + } > + snd_printk(KERN_INFO "firmware size=0x%x\n",fw_entry->size); > + if (fw_entry->size != 0x133a4) { > + snd_printk(KERN_INFO "firmware: %s wrong size.\n",filename); Ditto. > static int snd_emu10k1_emu1212m_init(struct snd_emu10k1 * emu) > { > unsigned int i; > - int tmp; > + int tmp,tmp2; > + int reg; > + int err; > + const char *hana_filename = "emu/hana.fw"; > + const char *dock_filename = "emu/audio_dock.fw"; > > snd_printk(KERN_ERR "emu1212m: Special config.\n"); Is this really an error? (snip) > + /* ID, should read & 0x7f = 0x55. (Bit 7 is the IRQ bit) */ > + snd_emu1212m_fpga_read(emu, EMU_HANA_ID, ® ); > + snd_printk("reg1=0x%x\n",reg); Please use snd_printdd or any other debug print functions. > + /* Enable 48Volt power to Audio Dock */ > + snd_emu1212m_fpga_write(emu, EMU_HANA_DOCK_PWR, EMU_HANA_DOCK_PWR_ON ); > + > + snd_emu1212m_fpga_read(emu, EMU_HANA_OPTION_CARDS, ® ); > + snd_printk(KERN_INFO "emu1212m: Card options=0x%x\n",reg); > + snd_emu1212m_fpga_read(emu, EMU_HANA_OPTION_CARDS, ® ); > + snd_printk(KERN_INFO "emu1212m: Card options=0x%x\n",reg); > + snd_emu1212m_fpga_read(emu, EMU_HANA_OPTICAL_TYPE, &tmp ); > + /* ADAT input. */ > + snd_emu1212m_fpga_write(emu, EMU_HANA_OPTICAL_TYPE, 0x01 ); > + snd_emu1212m_fpga_read(emu, EMU_HANA_DOCK_PADS, &tmp ); > + /* Set no attenuation on Audio Dock pads. */ > + snd_emu1212m_fpga_write(emu, EMU_HANA_DOCK_PADS, 0x00 ); > + snd_emu1212m_fpga_read(emu, EMU_HANA_DOCK_MISC, &tmp ); > + /* Unmute Audio dock DACs, Headphone source DAC-4. */ > + snd_emu1212m_fpga_write(emu, EMU_HANA_DOCK_MISC, 0x30 ); > + snd_emu1212m_fpga_write(emu, EMU_HANA_DOCK_LEDS_2, 0x12 ); > + snd_emu1212m_fpga_read(emu, EMU_HANA_UNKNOWN13, &tmp ); In future version, we may optimize this using array and loop. Not necessarily in this patch, though. (snip) > + /* FIXME: The loading of this should be able to happen any time, > + * as the user can plug/unplug it at any time > + */ > + if (reg & (EMU_HANA_OPTION_DOCK_ONLINE | EMU_HANA_OPTION_DOCK_OFFLINE) ) { > + /* Audio Dock attached */ > + /* Return to Audio Dock programming mode */ > + snd_printk(KERN_INFO "emu1212m: Loading Audio Dock Firmware\n"); > + snd_emu1212m_fpga_write(emu, EMU_HANA_FPGA_CONFIG, EMU_HANA_FPGA_CONFIG_AUDIODOCK ); > + if ((err = snd_emu1212m_load_firmware(emu, dock_filename)) != 0) { > + return err; > + } > + snd_printk(KERN_INFO "emu1212m: Audio Dock Firmware loaded\n"); > + snd_emu1212m_fpga_write(emu, EMU_HANA_FPGA_CONFIG, 0 ); > + snd_emu1212m_fpga_read(emu, EMU_HANA_IRQ_STATUS, ® ); > + snd_printk(KERN_INFO "emu1212m: EMU_HANA+DOCK_IRQ_STATUS=0x%x\n",reg); > + /* ID, should read & 0x7f = 0x55 when FPGA programmed. */ > + snd_emu1212m_fpga_read(emu, EMU_HANA_ID, ® ); > + snd_printk(KERN_INFO "emu1212m: EMU_HANA+DOCK_ID=0x%x\n",reg); > + if (reg != 0x55) { > + /* FPGA failed to be programmed */ > + snd_printk(KERN_INFO "emu1212m: Loading Audio Dock Firmware file failed, reg=0x%x\n", reg); > + return 0; > + return -ENODEV; Invalid return. > diff -r 61bfe5b3a7ae pci/emu10k1/emupcm.c > --- a/pci/emu10k1/emupcm.c Fri Aug 25 13:11:26 2006 +0200 > +++ b/pci/emu10k1/emupcm.c Sat Jul 22 23:43:02 2006 +0100 > @@ -790,6 +790,7 @@ static int snd_emu10k1_capture_trigger(s > if (emu->audigy) { > snd_emu10k1_ptr_write(emu, A_FXWC1, 0, epcm->capture_cr_val); > snd_emu10k1_ptr_write(emu, A_FXWC2, 0, epcm->capture_cr_val2); > + printk("cr_val=0x%x, cr_val2=0x%x\n", epcm->capture_cr_val, epcm->capture_cr_val2); Use debug prints. > diff -r 61bfe5b3a7ae pci/emu10k1/emuproc.c > --- a/pci/emu10k1/emuproc.c Fri Aug 25 13:11:26 2006 +0200 > +++ b/pci/emu10k1/emuproc.c Thu Jul 13 17:23:20 2006 +0100 > @@ -28,6 +28,7 @@ > #include <sound/driver.h> > #include <linux/slab.h> > #include <linux/init.h> > +#include <linux/delay.h> /* For udelay */ Hmm, where is udelay() in this file? > @@ -531,6 +553,9 @@ int __devinit snd_emu10k1_proc_init(stru > { > struct snd_info_entry *entry; > #ifdef CONFIG_SND_DEBUG > + if (! snd_card_proc_new(emu->card, "emu1212m_regs", &entry)) { > + snd_info_set_text_ops(entry, emu, snd_emu_proc_emu1212m_reg_read); > + } Isn't this proc file only for emu1212? How about to add check of emu->card_capabilities->emu1212m? thanks, Takashi ------------------------------------------------------------------------- Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642 _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.sourceforge.net/lists/listinfo/alsa-devel
