* Re: [PATCH] Implement basic support for Creative emu1212m and emu1820m.
2006-08-25 18:18 [PATCH] Implement basic support for Creative emu1212m and emu1820m James Courtier-Dutton
@ 2006-08-28 10:51 ` Takashi Iwai
0 siblings, 0 replies; 2+ messages in thread
From: Takashi Iwai @ 2006-08-28 10:51 UTC (permalink / raw)
To: James Courtier-Dutton; +Cc: alsa-devel
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
^ permalink raw reply [flat|nested] 2+ messages in thread