Re: [PATCH] spin[un]locks revision on new cmpci driver (5.64)

[Alan Cox]

> The following patch fixes SMP hangs w/ cmpci v5.64 ( k244-ac17 ).

Let me suggest a different approach

> - -   spin_lock_irqsave(&s->lock, flags);
>   set_spdifout(s, rate);
> + spin_lock_irqsave(&s->lock, flags);

Split the various locked versions stuff stuff like set_adc_rate out as
__set_adc_rate which doesnt take the lock, and set_adc_rate which takes the
lock and calls the other one.

That is how several other drivers were done and avoids messy lock/unlocks
some of which in your changes I think introduce races

Ditto for enable_dac as the old code and a newer __enable_dac, as well
I suspect as __set_spdifout (although is that ever called by anyone not
holding the lock ???)

>   init_timer(&s->midi.timer);
>   s->midi.timer.expires =3D jiffies+1;
>   s->midi.timer.data =3D (unsigned long)s;
> +
> + spin_unlock_irqrestore(&s->lock, flags);
>   s->midi.timer.function =3D cm_midi_timer;
> + spin_lock_irqsave(&s->lock, flags);
> +

That one doesnt seem to be needed


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] spin[un]locks revision on new cmpci driver (5.64)

[Carlos E Gorges]

-BEGIN PGP SIGNED MESSAGE-
Hash: RIPEMD160


HI all,

Finally I discovered my problem :-)

The following patch fixes SMP hangs w/ cmpci v5.64 ( k244-ac17 ).

- --- linux-244ac/drivers/sound/cmpci.c Fri May 25 05:26:27 2001
+++ linux/drivers/sound/cmpci.c Fri May 25 20:14:49 2001
@@ -1,5 +1,4 @@
 /
*/
- -
 /*
  *  cmpci.c  --  C-Media PCI audio driver.
  *
@@ -76,6 +75,10 @@
  *was calling prog_dmabuf with s->lock held, call missing
  *unlock_kernel in cm_midi_release
  *
+ *Fri May 25 2001 - Carlos Eduardo Gorges <[EMAIL PROTECTED]>
+ *- some drivers cleanups
+ *- spin[un]locks revision ( fix SMP support )
+ *
  */
 
 /
*/
@@ -226,10 +229,6 @@
 
 #define SND_DEV_DSP16   5 
 
- -#define  set_dac1_rate   set_adc_rate
- -#define  stop_dac1   stop_adc
- -#define  get_dmadac1 get_dmaadc
- -
 /* - */
 
 struct cm_state {
@@ -342,7 +341,6 @@
 /* - */
 
 static struct cm_state *devs;
- -static struct cm_state *devaudio;
 static unsigned long wavetable_mem;
 
 /* - */
@@ -577,8 +575,8 @@
 {
unsigned long flags;
 
- - spin_lock_irqsave(&s->lock, flags);
set_spdifout(s, rate);
+   spin_lock_irqsave(&s->lock, flags);
/* enable AC3 */
if (rate == 48000 || rate == 44100) {
// mute DAC
@@ -697,7 +695,7 @@
if (s->curr_channels <=  2)
set_spdifout(s, rate);
if (s->status & DO_DUAL_DAC)
- - set_dac1_rate(s, rate);
+   set_adc_rate(s, rate);
 }
 
 /* - */
@@ -759,12 +757,16 @@
 
 extern inline void enable_dac(struct cm_state *s)
 {
+   unsigned long flags;
+
+   spin_lock_irqsave(&s->lock, flags);
if (!(s->enable & CM_ENABLE_CH1)) {
/* enable channel */
s->enable |= CM_ENABLE_CH1;
outb(s->enable, s->iobase + CODEC_CMI_FUNCTRL0 + 2);
}
maskb(s->iobase + CODEC_CMI_FUNCTRL0, ~8, 0);
+   spin_unlock_irqrestore(&s->lock, flags);
if (s->status & DO_DUAL_DAC)
enable_adc(s);
 }
@@ -794,7 +796,7 @@
}
spin_unlock_irqrestore(&s->lock, flags);
if (s->status & DO_DUAL_DAC)
- - stop_dac1(s);
+   stop_adc(s);
 }
 
 static void start_adc(struct cm_state *s)
@@ -819,7 +821,9 @@
if ((s->dma_adc.mapped || s->dma_adc.count > 0) && s->dma_adc.ready) {
/* enable interrupt */
 // maskb(s->iobase + CODEC_CMI_INT_HLDCLR + 2, ~0, 1);
- - enable_dac(s);
+   spin_unlock_irqrestore(&s->lock, flags);
+   enable_dac(s);
+   spin_lock_irqsave(&s->lock, flags);
}
spin_unlock_irqrestore(&s->lock, flags);
 }  
@@ -832,7 +836,9 @@
if ((s->dma_dac.mapped || s->dma_dac.count > 0) && s->dma_dac.ready) {
/* enable interrupt */
maskb(s->iobase + CODEC_CMI_INT_HLDCLR + 2, ~0, 2);
+   spin_unlock_irqrestore(&s->lock, flags);
enable_dac(s);
+   spin_lock_irqsave(&s->lock, flags);
}
spin_unlock_irqrestore(&s->lock, flags);
if (s->status & DO_DUAL_DAC)
@@ -848,7 +854,11 @@
spin_lock_irqsave(&s->lock, flags);
if ((channels > 2) && (channels <= s->max_channels)
 && (((s->fmt >> CM_CFMT_DACSHIFT) & CM_CFMT_MASK) == (CM_CFMT_STEREO | 
CM_CFMT_16BIT))) {
+
+   spin_unlock_irqrestore(&s->lock, flags);
set_spdifout(s, 0);
+   spin_lock_irqsave(&s->lock, flags);
+
if (s->capability & CAN_MULTI_CH_HW) {
// NXCHG
maskb(s->iobase + CODEC_CMI_LEGACY_CTRL + 3, ~0, 0x80);
@@ -880,8 +890,11 @@
fmts |= CM_CFMT_16BIT << CM_CFMT_ADCSHIFT;
fmts |= CM_CFMT_STEREO << CM_CFMT_DACSHIFT;
fmts |= CM_CFMT_STEREO << CM_CFMT_ADCSHIFT;
+
+   spin_unlock_irqrestore(&s->lock, flags);
set_fmt(s, fmtm, fmts);
- - set_dac1_rate(s, s->ratedac);
+   set_adc_rate(s, s->ratedac);
+   spin_lock_irqsave(&s->lock, flags);
}
// N4SPK3D, disable 4 speaker mode (analog duplicate)
if (s->speakers > 2)
@@ -925,7 +938,6 @@
db->mapped = db->ready = 0;
 }
 
- -
 /* Ch1 is used for playback, Ch0 is used for recording */
 
 static int prog_dmabuf(struct cm_state *s, unsigned rec)
@@ -939,7 +951,6 @@
unsigned char fmt;
unsigned long flags;
 
- - spin_lock_irqsave(&s->lock, flags);
fmt = s