Messages in this thread Patch in this message | | | Date | Mon, 24 Jun 2002 08:51:30 +0300 | From | Muli Ben-Yehuda <> | Subject | PATCH: drivers/sound/trident.c [1/2] debug cleanup |
| |
Hello,
Here is a patch to drivers/sound/trident.c to make it adher to the common coding style in the kernel. I'm chasing an elusive bug in it, and cleaning it up as I go along. This patch implements a debug macro dependant on DEBUG and protected by do {} while (0), and replaces the various #ifdef DEBUG .. #endif statements littered in the code with it.
Patch was sent to maintainer, who replied that he no longer maintains the driver.
Patch against 2.4.19pre9.
diff -Naur --exclude-from /home/mulix/work/dontdiff linux-2.4.19-pre9/drivers/sound/trident.c linux-2.4.19-pre9-mx/drivers/sound/trident.c --- linux-2.4.19-pre9/drivers/sound/trident.c Wed May 29 10:47:59 2002 +++ linux-2.4.19-pre9-mx/drivers/sound/trident.c Fri Jun 21 15:25:25 2002 @@ -36,6 +36,10 @@ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. * * History + * v0.14.9e + * June 21 2002 Muli Ben-Yehuda <mulix@actcom.co.il> + * use a debug macro instead of #ifdef CONFIG_DEBUG, trim to 80 columns + * per line, use 'do {} while (0)' in statement macros. * v0.14.9d * October 8 2001 Arnaldo Carvalho de Melo <acme@conectiva.com.br> * use set_current_state, properly release resources on failure in @@ -180,7 +184,7 @@ #include <linux/pm.h> -#define DRIVER_VERSION "0.14.9d" +#define DRIVER_VERSION "0.14.9e" /* magic numbers to protect our data structures */ #define TRIDENT_CARD_MAGIC 0x5072696E /* "Prin" */ @@ -198,7 +202,13 @@ /* minor number of /dev/swmodem (temporary, experimental) */ #define SND_DEV_SWMODEM 7 -static const unsigned ali_multi_channels_5_1[] = { /*ALI_SURR_LEFT_CHANNEL, ALI_SURR_RIGHT_CHANNEL,*/ ALI_CENTER_CHANNEL, ALI_LEF_CHANNEL, ALI_SURR_LEFT_CHANNEL, ALI_SURR_RIGHT_CHANNEL}; +static const unsigned ali_multi_channels_5_1[] = { + /*ALI_SURR_LEFT_CHANNEL, ALI_SURR_RIGHT_CHANNEL,*/ + ALI_CENTER_CHANNEL, + ALI_LEF_CHANNEL, + ALI_SURR_LEFT_CHANNEL, + ALI_SURR_RIGHT_CHANNEL +}; static const unsigned sample_size[] = { 1, 2, 2, 4 }; static const unsigned sample_shift[] = { 0, 1, 1, 2 }; @@ -288,7 +298,7 @@ } dmabuf; - /* 5.1channels */ + /* 5.1 channels */ struct trident_state *other_states[4]; int multi_channels_adjust_count; unsigned chans_num; @@ -431,8 +441,11 @@ static void ali_disable_spdif_in(struct trident_card *card); static void ali_disable_special_channel(struct trident_card *card, int ch); static void ali_setup_spdif_out(struct trident_card *card, int flag); -static int ali_write_5_1(struct trident_state *state, const char *buffer,int cnt_for_multi_channel, unsigned int *copy_count, unsigned int *state_cnt); -static int ali_allocate_other_states_resources(struct trident_state *state, int chan_nums); +static int ali_write_5_1(struct trident_state *state, const char *buffer, + int cnt_for_multi_channel, unsigned int *copy_count, + unsigned int *state_cnt); +static int ali_allocate_other_states_resources(struct trident_state *state, + int chan_nums); static void ali_free_other_states_resources(struct trident_state *state); @@ -443,22 +456,29 @@ unsigned mixer_regs[ALI_MIXER_REGS]; } ali_registers; -#define seek_offset(dma_ptr, buffer, cnt, offset, copy_count) (dma_ptr) += (offset); \ - (buffer) += (offset); \ - (cnt) -= (offset); \ - (copy_count) += (offset); - -#define lock_set_fmt(state) {spin_lock_irqsave(&state->card->lock, flags); \ - if (state->fmt_flag) { \ - spin_unlock_irqrestore(&state->card->lock, flags); \ - return -EFAULT; \ - } \ - state->fmt_flag = 1; \ - spin_unlock_irqrestore(&state->card->lock, flags);} +#define seek_offset(dma_ptr, buffer, cnt, offset, copy_count) do { \ + (dma_ptr) += (offset); \ + (buffer) += (offset); \ + (cnt) -= (offset); \ + (copy_count) += (offset); \ +} while (0) + + +#define lock_set_fmt(state) do { \ + spin_lock_irqsave(&state->card->lock, flags); \ + if (state->fmt_flag) { \ + spin_unlock_irqrestore(&state->card->lock, flags); \ + return -EFAULT; \ + } \ + state->fmt_flag = 1; \ + spin_unlock_irqrestore(&state->card->lock, flags); \ +} while (0) -#define unlock_set_fmt(state) {spin_lock_irqsave(&state->card->lock, flags); \ - state->fmt_flag = 0; \ - spin_unlock_irqrestore(&state->card->lock, flags);} +#define unlock_set_fmt(state) do { \ + spin_lock_irqsave(&state->card->lock, flags); \ + state->fmt_flag = 0; \ + spin_unlock_irqrestore(&state->card->lock, flags); \ +} while (0) static int trident_enable_loop_interrupts(struct trident_card * card) { @@ -483,10 +503,9 @@ outl(global_control, TRID_REG(card, T4D_LFO_GC_CIR)); -#ifdef DEBUG - printk("trident: Enable Loop Interrupts, globctl = 0x%08X\n", - inl(TRID_REG(card, T4D_LFO_GC_CIR))); -#endif + TRDBG("trident: Enable Loop Interrupts, globctl = 0x%08X\n", + inl(TRID_REG(card, T4D_LFO_GC_CIR))); + return (TRUE); } @@ -498,10 +517,9 @@ global_control &= ~(ENDLP_IE | MIDLP_IE); outl(global_control, TRID_REG(card, T4D_LFO_GC_CIR)); -#ifdef DEBUG - printk("trident: Disabled Loop Interrupts, globctl = 0x%08X\n", - global_control); -#endif + TRDBG("trident: Disabled Loop Interrupts, globctl = 0x%08X\n", + global_control); + return (TRUE); } @@ -517,9 +535,9 @@ #ifdef DEBUG reg = inl(TRID_REG(card, addr)); - printk("trident: enabled IRQ on channel %d, %s = 0x%08x(addr:%X)\n", - channel, addr==T4D_AINTEN_B? "AINTEN_B":"AINTEN_A",reg,addr); -#endif + TRDBG("trident: enabled IRQ on channel %d, %s = 0x%08x(addr:%X)\n", + channel, addr==T4D_AINTEN_B? "AINTEN_B":"AINTEN_A",reg,addr); +#endif /* DEBUG */ } static void trident_disable_voice_irq(struct trident_card * card, unsigned int channel) @@ -537,9 +555,9 @@ #ifdef DEBUG reg = inl(TRID_REG(card, addr)); - printk("trident: disabled IRQ on channel %d, %s = 0x%08x(addr:%X)\n", - channel, addr==T4D_AINTEN_B? "AINTEN_B":"AINTEN_A",reg,addr); -#endif + TRDBG("trident: disabled IRQ on channel %d, %s = 0x%08x(addr:%X)\n", + channel, addr==T4D_AINTEN_B? "AINTEN_B":"AINTEN_A",reg,addr); +#endif /* DEBUG */ } static void trident_start_voice(struct trident_card * card, unsigned int channel) @@ -550,15 +568,15 @@ #ifdef DEBUG u32 reg; -#endif +#endif /* DEBUG */ outl(mask, TRID_REG(card, addr)); -#ifdef DEBUG +#ifdef DEBUG reg = inl(TRID_REG(card, addr)); - printk("trident: start voice on channel %d, %s = 0x%08x(addr:%X)\n", - channel, addr==T4D_START_B? "START_B":"START_A",reg,addr); -#endif + TRDBG("trident: start voice on channel %d, %s = 0x%08x(addr:%X)\n", + channel, addr==T4D_START_B? "START_B":"START_A",reg,addr); +#endif /* DEBUG */ } static void trident_stop_voice(struct trident_card * card, unsigned int channel) @@ -569,15 +587,15 @@ #ifdef DEBUG u32 reg; -#endif +#endif /* DEBUG */ outl(mask, TRID_REG(card, addr)); #ifdef DEBUG reg = inl(TRID_REG(card, addr)); - printk("trident: stop voice on channel %d, %s = 0x%08x(addr:%X)\n", - channel, addr==T4D_STOP_B? "STOP_B":"STOP_A",reg,addr); -#endif + TRDBG("trident: stop voice on channel %d, %s = 0x%08x(addr:%X)\n", + channel, addr==T4D_STOP_B? "STOP_B":"STOP_A",reg,addr); +#endif /* DEBUG */ } static u32 trident_get_interrupt_mask (struct trident_card * card, unsigned int channel) @@ -594,9 +612,9 @@ #ifdef DEBUG if (reg & mask) - printk("trident: channel %d has interrupt, %s = 0x%08x\n", - channel,reg==T4D_AINT_B? "AINT_B":"AINT_A", reg); -#endif + TRDBG("trident: channel %d has interrupt, %s = 0x%08x\n", + channel,reg==T4D_AINT_B? "AINT_B":"AINT_A", reg); +#endif /* DEBUG */ return (reg & mask) ? TRUE : FALSE; } @@ -612,9 +630,9 @@ #ifdef DEBUG reg = inl(TRID_REG(card, T4D_AINT_B)); - printk("trident: Ack channel %d interrupt, AINT_B = 0x%08x\n", - channel, reg); -#endif + TRDBG("trident: Ack channel %d interrupt, AINT_B = 0x%08x\n", + channel, reg); +#endif /* DEBUG */ } static struct trident_channel * trident_alloc_pcm_channel(struct trident_card *card) @@ -862,9 +880,7 @@ trident_write_voice_regs(state); -#ifdef DEBUG - printk("trident: called trident_set_dac_rate : rate = %d\n", rate); -#endif + TRDBG("trident: called trident_set_dac_rate : rate = %d\n", rate); return rate; } @@ -884,9 +900,8 @@ trident_write_voice_regs(state); -#ifdef DEBUG - printk("trident: called trident_set_adc_rate : rate = %d\n", rate); -#endif + TRDBG("trident: called trident_set_adc_rate : rate = %d\n", rate); + return rate; } @@ -928,11 +943,11 @@ if (dmabuf->fmt & TRIDENT_FMT_STEREO) /* stereo */ channel->control |= CHANNEL_STEREO; -#ifdef DEBUG - printk("trident: trident_play_setup, LBA = 0x%08x, " - "Delta = 0x%08x, ESO = 0x%08x, Control = 0x%08x\n", - channel->lba, channel->delta, channel->eso, channel->control); -#endif + + TRDBG("trident: trident_play_setup, LBA = 0x%08x, " + "Delta = 0x%08x, ESO = 0x%08x, Control = 0x%08x\n", + channel->lba, channel->delta, channel->eso, channel->control); + trident_write_voice_regs(state); } @@ -1014,11 +1029,11 @@ if (dmabuf->fmt & TRIDENT_FMT_STEREO) /* stereo */ channel->control |= CHANNEL_STEREO; -#ifdef DEBUG - printk("trident: trident_rec_setup, LBA = 0x%08x, " - "Delat = 0x%08x, ESO = 0x%08x, Control = 0x%08x\n", - channel->lba, channel->delta, channel->eso, channel->control); -#endif + + TRDBG("trident: trident_rec_setup, LBA = 0x%08x, " + "Delat = 0x%08x, ESO = 0x%08x, Control = 0x%08x\n", + channel->lba, channel->delta, channel->eso, channel->control); + trident_write_voice_regs(state); } @@ -1051,11 +1066,11 @@ return 0; } -#ifdef DEBUG - printk("trident: trident_get_dma_addr: chip reported channel: %d, " - "cso = 0x%04x\n", - dmabuf->channel->num, cso); -#endif + + TRDBG("trident: trident_get_dma_addr: chip reported channel: %d, " + "cso = 0x%04x\n", + dmabuf->channel->num, cso); + /* ESO and CSO are in units of Samples, convert to byte offset */ cso <<= sample_shift[dmabuf->fmt]; @@ -1170,10 +1185,8 @@ if (!rawbuf) return -ENOMEM; -#ifdef DEBUG - printk("trident: allocated %ld (order = %d) bytes at %p\n", - PAGE_SIZE << order, order, rawbuf); -#endif + TRDBG("trident: allocated %ld (order = %d) bytes at %p\n", + PAGE_SIZE << order, order, rawbuf); dmabuf->ready = dmabuf->mapped = 0; dmabuf->rawbuf = rawbuf; @@ -1304,12 +1317,10 @@ /* set the ready flag for the dma buffer */ dmabuf->ready = 1; -#ifdef DEBUG - printk("trident: prog_dmabuf(%d), sample rate = %d, format = %d, numfrag = %d, " - "fragsize = %d dmasize = %d\n", - dmabuf->channel->num, dmabuf->rate, dmabuf->fmt, dmabuf->numfrag, - dmabuf->fragsize, dmabuf->dmasize); -#endif + TRDBG("trident: prog_dmabuf(%d), sample rate = %d, format = %d, numfrag = %d, " + "fragsize = %d dmasize = %d\n", + dmabuf->channel->num, dmabuf->rate, dmabuf->fmt, dmabuf->numfrag, + dmabuf->fragsize, dmabuf->dmasize); } unlock_set_fmt(state); return 0; @@ -1657,22 +1668,21 @@ /* Update the pointers for all channels we are running. */ /* FIXED: read interrupt status only once */ irq_status=inl(TRID_REG(card, T4D_AINT_A) ); -#ifdef DEBUG - printk("cyber_address_interrupt: irq_status 0x%X\n",irq_status); -#endif + + TRDBG("cyber_address_interrupt: irq_status 0x%X\n",irq_status); + for (i = 0; i < NR_HW_CH; i++) { if (irq_status & ( 1 << (31 - i)) ) { /* clear bit by writing a 1, zeroes are ignored */ outl( (1 <<(31-i)), TRID_REG(card, T4D_AINT_A)); -#ifdef DEBUG - printk("cyber_interrupt: channel %d\n", 31-i); -#endif + TRDBG("cyber_interrupt: channel %d\n", 31-i); + if ((state = card->states[i]) != NULL) { trident_update_ptr(state); } else { - printk("cyber5050: spurious channel irq %d.\n", + printk(KERN_WARNING "cyber5050: spurious channel irq %d.\n", 31 - i); trident_stop_voice(card, 31 - i); trident_disable_voice_irq(card, 31 - i); @@ -1690,9 +1700,7 @@ spin_lock(&card->lock); event = inl(TRID_REG(card, T4D_MISCINT)); -#ifdef DEBUG - printk("trident: trident_interrupt called, MISCINT = 0x%08x\n", event); -#endif + TRDBG("trident: trident_interrupt called, MISCINT = 0x%08x\n", event); if (event & ADDRESS_IRQ) { card->address_interrupt(card); @@ -1730,9 +1738,7 @@ unsigned swptr; int cnt; -#ifdef DEBUG - printk("trident: trident_read called, count = %d\n", count); -#endif + TRDBG("trident: trident_read called, count = %d\n", count); VALIDATE_STATE(state); if (ppos != &file->f_pos) @@ -1786,12 +1792,11 @@ which results in a (potential) buffer overrun. And worse, there is NOTHING we can do to prevent it. */ if (!interruptible_sleep_on_timeout(&dmabuf->wait, tmo)) { -#ifdef DEBUG - printk(KERN_ERR "trident: recording schedule timeout, " - "dmasz %u fragsz %u count %i hwptr %u swptr %u\n", - dmabuf->dmasize, dmabuf->fragsize, dmabuf->count, - dmabuf->hwptr, dmabuf->swptr); -#endif + TRDBG(KERN_ERR "trident: recording schedule timeout, " + "dmasz %u fragsz %u count %i hwptr %u swptr %u\n", + dmabuf->dmasize, dmabuf->fragsize, dmabuf->count, + dmabuf->hwptr, dmabuf->swptr); + /* a buffer overrun, we delay the recovery until next time the while loop begin and we REALLY have space to record */ } @@ -1845,9 +1850,8 @@ unsigned int state_cnt; unsigned int copy_count; -#ifdef DEBUG - printk("trident: trident_write called, count = %d\n", count); -#endif + TRDBG("trident: trident_write called, count = %d\n", count); + VALIDATE_STATE(state); if (ppos != &file->f_pos) return -ESPIPE; @@ -1915,12 +1919,11 @@ which results in a (potential) buffer underrun. And worse, there is NOTHING we can do to prevent it. */ if (!interruptible_sleep_on_timeout(&dmabuf->wait, tmo)) { -#ifdef DEBUG - printk(KERN_ERR "trident: playback schedule timeout, " - "dmasz %u fragsz %u count %i hwptr %u swptr %u\n", - dmabuf->dmasize, dmabuf->fragsize, dmabuf->count, - dmabuf->hwptr, dmabuf->swptr); -#endif + TRDBG(KERN_ERR "trident: playback schedule timeout, " + "dmasz %u fragsz %u count %i hwptr %u swptr %u\n", + dmabuf->dmasize, dmabuf->fragsize, dmabuf->count, + dmabuf->hwptr, dmabuf->swptr); + /* a buffer underrun, we delay the recovery until next time the while loop begin and we REALLY have data to play */ } @@ -2099,10 +2102,8 @@ VALIDATE_STATE(state); mapped = ((file->f_mode & FMODE_WRITE) && dmabuf->mapped) || ((file->f_mode & FMODE_READ) && dmabuf->mapped); -#ifdef DEBUG - printk("trident: trident_ioctl, command = %2d, arg = 0x%08x\n", - _IOC_NR(cmd), arg ? *(int *)arg : 0); -#endif + TRDBG("trident: trident_ioctl, command = %2d, arg = 0x%08x\n", + _IOC_NR(cmd), arg ? *(int *)arg : 0); switch (cmd) { @@ -2665,10 +2666,8 @@ state->open_mode |= file->f_mode & (FMODE_READ | FMODE_WRITE); up(&card->open_sem); -#ifdef DEBUG - printk(KERN_ERR "trident: open virtual channel %d, hard channel %d\n", + TRDBG("trident: open virtual channel %d, hard channel %d\n", state->virt, dmabuf->channel->num); -#endif return 0; } @@ -2690,10 +2689,8 @@ drain_dac(state, file->f_flags & O_NONBLOCK); } -#ifdef DEBUG - printk(KERN_ERR "trident: closing virtual channel %d, hard channel %d\n", - state->virt, dmabuf->channel->num); -#endif + TRDBG("trident: closing virtual channel %d, hard channel %d\n", + state->virt, dmabuf->channel->num); /* stop DMA state machine and free DMA buffers/channels */ down(&card->open_sem); diff -Naur --exclude-from /home/mulix/work/dontdiff linux-2.4.19-pre9/drivers/sound/trident.h linux-2.4.19-pre9-mx/drivers/sound/trident.h --- linux-2.4.19-pre9/drivers/sound/trident.h Sat Nov 10 00:07:41 2001 +++ linux-2.4.19-pre9-mx/drivers/sound/trident.h Fri Jun 21 15:04:42 2002 @@ -358,5 +358,16 @@ return r; } -#endif /* __TRID4DWAVE_H */ +#ifdef DEBUG + +#define TRDBG(msg, args...) do { \ + printk(KERN_DEBUG msg , ##args ); \ +} while (0) + +#else /* !defined(DEBUG) */ +#define TRDBG(msg, args...) do { } while (0) + +#endif /* DEBUG */ + +#endif /* __TRID4DWAVE_H */[unhandled content-type:application/pgp-signature] | |