All of lore.kernel.org
 help / color / mirror / Atom feed
* sparc dbri: SMP fixes
@ 2006-09-01  7:11 Krzysztof Helt
  2006-09-01 10:30 ` Takashi Iwai
  0 siblings, 1 reply; 4+ messages in thread
From: Krzysztof Helt @ 2006-09-01  7:11 UTC (permalink / raw)
  To: alsa-devel

[-- Attachment #1: Type: text/plain, Size: 220 bytes --]

From: Krzysztof Helt (krzysztof.h1@wp.pl)

The dbri driver hangs when used in kernel compiled with SMP
support due to inproper locking. The patch fixes it.

Signed-off-by: Krzysztof Helt (krzysztof.h1@wp.pl)
--- 

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: dbri-patch15.diff --]
[-- Type: text/x-patch; name="dbri-patch15.diff", Size: 10544 bytes --]

--- alsa-driver-1.0.12rc2/alsa-kernel/sparc/dbri.c	2006-08-25 23:57:49.000000000 +0200
+++ linux-2.6.17a/sound/sparc/dbri.c	2006-08-31 22:44:01.000000000 +0200
@@ -635,10 +635,16 @@ to send them to the DBRI.
 static void dbri_cmdwait(struct snd_dbri *dbri)
 {
 	int maxloops = MAXLOOPS;
+	unsigned long flags;
 
 	/* Delay if previous commands are still being processed */
-	while ((--maxloops) > 0 && (sbus_readl(dbri->regs + REG0) & D_P))
+	spin_lock_irqsave(&dbri->lock, flags);
+	while ((--maxloops) > 0 && (sbus_readl(dbri->regs + REG0) & D_P)) {
+		spin_unlock_irqrestore(&dbri->lock, flags);
 		msleep_interruptible(1);
+		spin_lock_irqsave(&dbri->lock, flags);
+	}
+	spin_unlock_irqrestore(&dbri->lock, flags);
 
 	if (maxloops == 0) {
 		printk(KERN_ERR "DBRI: Chip never completed command buffer\n");
@@ -672,10 +678,10 @@ static s32 *dbri_cmdlock(struct snd_dbri
  * The JUMP cmd points to the new cmd string.
  * It also releases the cmdlock spinlock.
  */
-static void dbri_cmdsend(struct snd_dbri * dbri, s32 * cmd,int len)
+static void dbri_cmdsend(struct snd_dbri * dbri, s32 * cmd,int len,int lock)
 {
 	s32 tmp, addr;
-	unsigned long flags;
+	unsigned long flags = 0;
 	static int wait_id = 0;
 
 	wait_id++;
@@ -706,12 +712,14 @@ static void dbri_cmdsend(struct snd_dbri
 	}
 #endif
 
-	spin_lock_irqsave(&dbri->lock, flags);
+	if (!lock)
+		spin_lock_irqsave(&dbri->lock, flags);
 	/* Reread the last command */
 	tmp = sbus_readl(dbri->regs + REG0);
 	tmp |= D_P;
 	sbus_writel(tmp, dbri->regs + REG0);
-	spin_unlock_irqrestore(&dbri->lock, flags);
+	if (!lock)
+		spin_unlock_irqrestore(&dbri->lock, flags);
 
 	dbri->cmdptr = cmd;
 	spin_unlock(&dbri->cmdlock);
@@ -777,9 +785,9 @@ static void dbri_initialize(struct snd_d
 	dma_addr = dbri->dma_dvma + dbri_dma_off(cmd, 0);
 	sbus_writel(dma_addr, dbri->regs + REG8);
 	spin_unlock(&dbri->cmdlock);
-	dbri_cmdwait(dbri);
 
 	spin_unlock_irqrestore(&dbri->lock, flags);
+	dbri_cmdwait(dbri);
 }
 
 /*
@@ -827,7 +835,7 @@ static void reset_pipe(struct snd_dbri *
 	*(cmd++) = DBRI_CMD(D_SDP, 0, sdp | D_SDP_C | D_SDP_P);
 	*(cmd++) = 0;
 	*(cmd++) = DBRI_CMD(D_PAUSE, 0, 0);
-	dbri_cmdsend(dbri, cmd, 3);
+	dbri_cmdsend(dbri, cmd, 3, 1);
 
 	desc = dbri->pipes[pipe].first_desc;
 	if ( desc >= 0)
@@ -840,6 +848,9 @@ static void reset_pipe(struct snd_dbri *
 	dbri->pipes[pipe].first_desc = -1;
 }
 
+/*
+ * Lock must be held before calling this.
+ */
 static void setup_pipe(struct snd_dbri * dbri, int pipe, int sdp)
 {
 	if (pipe < 0 || pipe > DBRI_MAX_PIPE) {
@@ -866,6 +877,9 @@ static void setup_pipe(struct snd_dbri *
 	reset_pipe(dbri, pipe);
 }
 
+/*
+ * Lock must be held before calling this.
+ */
 static void link_time_slot(struct snd_dbri * dbri, int pipe,
 			   int prevpipe, int nextpipe,
 			   int length, int cycle)
@@ -917,9 +931,10 @@ static void link_time_slot(struct snd_db
 	}
 	*(cmd++) = DBRI_CMD(D_PAUSE, 0, 0);
 
-	dbri_cmdsend(dbri, cmd, 4);
+	dbri_cmdsend(dbri, cmd, 4, 1);
 }
 
+#if 0
 static void unlink_time_slot(struct snd_dbri * dbri, int pipe,
 			     enum in_or_out direction, int prevpipe,
 			     int nextpipe)
@@ -952,6 +967,7 @@ static void unlink_time_slot(struct snd_
 
 	dbri_cmdsend(dbri, cmd, 4);
 }
+#endif
 
 /* xmit_fixed() / recv_fixed()
  *
@@ -965,7 +981,9 @@ static void unlink_time_slot(struct snd_
  * the actual time slot is.  The interrupt handler takes care of bit
  * ordering and alignment.  An 8-bit time slot will always end up
  * in the low-order 8 bits, filled either MSB-first or LSB-first,
- * depending on the settings passed to setup_pipe()
+ * depending on the settings passed to setup_pipe().
+ *
+ * Lock must not be held before calling it.
  */
 static void xmit_fixed(struct snd_dbri * dbri, int pipe, unsigned int data)
 {
@@ -1002,8 +1020,9 @@ static void xmit_fixed(struct snd_dbri *
 	*(cmd++) = data;
 	*(cmd++) = DBRI_CMD(D_PAUSE, 0, 0);
 
-	dbri_cmdsend(dbri, cmd, 3);
+	dbri_cmdsend(dbri, cmd, 3, 0);
 	dbri_cmdwait(dbri);
+
 }
 
 static void recv_fixed(struct snd_dbri * dbri, int pipe, volatile __u32 * ptr)
@@ -1039,6 +1058,8 @@ static void recv_fixed(struct snd_dbri *
  * be spread across multiple descriptors.
  *
  * All descriptors create a ring buffer.
+ *
+ * Lock must be held before calling this.
  */
 static int setup_descs(struct snd_dbri * dbri, int streamno, unsigned int period)
 {
@@ -1186,6 +1207,9 @@ multiplexed serial interface which the D
 
 enum master_or_slave { CHImaster, CHIslave };
 
+/*
+ * Lock must not be held before calling it.
+ */
 static void reset_chi(struct snd_dbri * dbri, enum master_or_slave master_or_slave,
 		      int bits_per_frame)
 {
@@ -1201,7 +1225,7 @@ static void reset_chi(struct snd_dbri * 
 	*(cmd++) = D_TS_ANCHOR | D_TS_NEXT(16);
 	*(cmd++) = D_TS_ANCHOR | D_TS_NEXT(16);
 	*(cmd++) = DBRI_CMD(D_PAUSE, 0, 0);
-	dbri_cmdsend(dbri, cmd, 4);
+	dbri_cmdsend(dbri, cmd, 4, 1);
 
 	dbri->pipes[16].sdp = 1;
 	dbri->pipes[16].nextpipe = 16;
@@ -1247,7 +1271,7 @@ static void reset_chi(struct snd_dbri * 
 	*(cmd++) = DBRI_CMD(D_CDM, 0, D_CDM_XCE | D_CDM_XEN | D_CDM_REN);
 	*(cmd++) = DBRI_CMD(D_PAUSE, 0, 0);
 
-	dbri_cmdsend(dbri, cmd, 4);
+	dbri_cmdsend(dbri, cmd, 4, 1);
 }
 
 /*
@@ -1258,9 +1282,14 @@ static void reset_chi(struct snd_dbri * 
 In the standard SPARC audio configuration, the CS4215 codec is attached
 to the DBRI via the CHI interface and few of the DBRI's PIO pins.
 
+ * Lock must not be held before calling it.
+
 */
 static void cs4215_setup_pipes(struct snd_dbri * dbri)
 {
+	unsigned long flags;
+
+	spin_lock_irqsave(&dbri->lock, flags);
 	/*
 	 * Data mode:
 	 * Pipe  4: Send timeslots 1-4 (audio data)
@@ -1284,6 +1313,7 @@ static void cs4215_setup_pipes(struct sn
 	setup_pipe(dbri, 17, D_SDP_FIXED | D_SDP_TO_SER | D_SDP_MSB);
 	setup_pipe(dbri, 18, D_SDP_FIXED | D_SDP_FROM_SER | D_SDP_MSB);
 	setup_pipe(dbri, 19, D_SDP_FIXED | D_SDP_FROM_SER | D_SDP_MSB);
+	spin_unlock_irqrestore(&dbri->lock, flags);
 
 	dbri_cmdwait(dbri);
 }
@@ -1358,6 +1388,7 @@ static void cs4215_open(struct snd_dbri 
 {
 	int data_width;
 	u32 tmp;
+	unsigned long flags;
 
 	dprintk(D_MM, "cs4215_open: %d channels, %d bits\n",
 		dbri->mm.channels, dbri->mm.precision);
@@ -1382,6 +1413,7 @@ static void cs4215_open(struct snd_dbri 
 	 * bits.  The CS4215, it seems, observes TSIN (the delayed signal)
 	 * even if it's the CHI master.  Don't ask me...
 	 */
+	spin_lock_irqsave(&dbri->lock, flags);
 	tmp = sbus_readl(dbri->regs + REG0);
 	tmp &= ~(D_C);		/* Disable CHI */
 	sbus_writel(tmp, dbri->regs + REG0);
@@ -1409,6 +1441,7 @@ static void cs4215_open(struct snd_dbri 
 	tmp = sbus_readl(dbri->regs + REG0);
 	tmp |= D_C;		/* Enable CHI */
 	sbus_writel(tmp, dbri->regs + REG0);
+	spin_unlock_irqrestore(&dbri->lock, flags);
 
 	cs4215_setdata(dbri, 0);
 }
@@ -1420,6 +1453,7 @@ static int cs4215_setctrl(struct snd_dbr
 {
 	int i, val;
 	u32 tmp;
+	unsigned long flags;
 
 	/* FIXME - let the CPU do something useful during these delays */
 
@@ -1456,6 +1490,7 @@ static int cs4215_setctrl(struct snd_dbr
 	 * done in hardware by a TI 248 that delays the DBRI->4215
 	 * frame sync signal by eight clock cycles.  Anybody know why?
 	 */
+	spin_lock_irqsave(&dbri->lock, flags);
 	tmp = sbus_readl(dbri->regs + REG0);
 	tmp &= ~D_C;		/* Disable CHI */
 	sbus_writel(tmp, dbri->regs + REG0);
@@ -1472,14 +1507,17 @@ static int cs4215_setctrl(struct snd_dbr
 	link_time_slot(dbri, 17, 16, 16, 32, dbri->mm.offset);
 	link_time_slot(dbri, 18, 16, 16, 8, dbri->mm.offset);
 	link_time_slot(dbri, 19, 18, 16, 8, dbri->mm.offset + 48);
+	spin_unlock_irqrestore(&dbri->lock, flags);
 
 	/* Wait for the chip to echo back CLB (Control Latch Bit) as zero */
 	dbri->mm.ctrl[0] &= ~CS4215_CLB;
 	xmit_fixed(dbri, 17, *(int *)dbri->mm.ctrl);
 
+	spin_lock_irqsave(&dbri->lock, flags);
 	tmp = sbus_readl(dbri->regs + REG0);
 	tmp |= D_C;		/* Enable CHI */
 	sbus_writel(tmp, dbri->regs + REG0);
+	spin_unlock_irqrestore(&dbri->lock, flags);
 
 	for (i = 10; ((dbri->mm.status & 0xe4) != 0x20); --i) {
 		msleep_interruptible(1);
@@ -1661,7 +1699,7 @@ static void xmit_descs(struct snd_dbri *
 					    dbri->pipes[info->pipe].sdp
 					    | D_SDP_P | D_SDP_EVERY | D_SDP_C);
 			*(cmd++) = dbri->dma_dvma + dbri_dma_off(desc, first_td);
-			dbri_cmdsend(dbri, cmd, 2);
+			dbri_cmdsend(dbri, cmd, 2, 1);
 
 			/* Reset our admin of the pipe. */
 			dbri->pipes[info->pipe].desc = first_td;
@@ -1682,12 +1720,13 @@ static void xmit_descs(struct snd_dbri *
 					    dbri->pipes[info->pipe].sdp
 					    | D_SDP_P | D_SDP_EVERY | D_SDP_C);
 			*(cmd++) = dbri->dma_dvma + dbri_dma_off(desc, first_td);
-			dbri_cmdsend(dbri, cmd, 2);
+			dbri_cmdsend(dbri, cmd, 2, 1);
 
 			/* Reset our admin of the pipe. */
 			dbri->pipes[info->pipe].desc = first_td;
 		}
 	}
+
 	spin_unlock_irqrestore(&dbri->lock, flags);
 }
 
@@ -2093,7 +2132,6 @@ static int snd_dbri_prepare(struct snd_p
 {
 	struct snd_dbri *dbri = snd_pcm_substream_chip(substream);
 	struct dbri_streaminfo *info = DBRI_STREAM(dbri, substream);
-	struct snd_pcm_runtime *runtime = substream->runtime;
 	int ret;
 
 	info->size = snd_pcm_lib_buffer_bytes(substream);
@@ -2234,7 +2270,6 @@ static int snd_cs4215_put_volume(struct 
 {
 	struct snd_dbri *dbri = snd_kcontrol_chip(kcontrol);
 	struct dbri_streaminfo *info = &dbri->stream_info[kcontrol->private_value];
-	unsigned long flags;
 	int changed = 0;
 
 	if (info->left_gain != ucontrol->value.integer.value[0]) {
@@ -2249,13 +2284,9 @@ static int snd_cs4215_put_volume(struct 
 		/* First mute outputs, and wait 1/8000 sec (125 us)
 		 * to make sure this takes.  This avoids clicking noises.
 		 */
-		spin_lock_irqsave(&dbri->lock, flags);
-
 		cs4215_setdata(dbri, 1);
 		udelay(125);
 		cs4215_setdata(dbri, 0);
-
-		spin_unlock_irqrestore(&dbri->lock, flags);
 	}
 	return changed;
 }
@@ -2302,7 +2333,6 @@ static int snd_cs4215_put_single(struct 
 				 struct snd_ctl_elem_value *ucontrol)
 {
 	struct snd_dbri *dbri = snd_kcontrol_chip(kcontrol);
-	unsigned long flags;
 	int elem = kcontrol->private_value & 0xff;
 	int shift = (kcontrol->private_value >> 8) & 0xff;
 	int mask = (kcontrol->private_value >> 16) & 0xff;
@@ -2335,13 +2365,9 @@ static int snd_cs4215_put_single(struct 
 		/* First mute outputs, and wait 1/8000 sec (125 us)
 		 * to make sure this takes.  This avoids clicking noises.
 		 */
-		spin_lock_irqsave(&dbri->lock, flags);
-
 		cs4215_setdata(dbri, 1);
 		udelay(125);
 		cs4215_setdata(dbri, 0);
-
-		spin_unlock_irqrestore(&dbri->lock, flags);
 	}
 	return changed;
 }

[-- Attachment #3: Type: text/plain, Size: 373 bytes --]

-------------------------------------------------------------------------
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

[-- Attachment #4: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: sparc dbri: SMP fixes
  2006-09-01  7:11 sparc dbri: SMP fixes Krzysztof Helt
@ 2006-09-01 10:30 ` Takashi Iwai
  2006-09-05 18:14   ` Krzysztof Helt
  0 siblings, 1 reply; 4+ messages in thread
From: Takashi Iwai @ 2006-09-01 10:30 UTC (permalink / raw)
  To: Krzysztof Helt; +Cc: alsa-devel

At Fri, 01 Sep 2006 09:11:05 +0200,
Krzysztof Helt wrote:
> 
> @@ -672,10 +678,10 @@ static s32 *dbri_cmdlock(struct snd_dbri
>   * The JUMP cmd points to the new cmd string.
>   * It also releases the cmdlock spinlock.
>   */
> -static void dbri_cmdsend(struct snd_dbri * dbri, s32 * cmd,int len)
> +static void dbri_cmdsend(struct snd_dbri * dbri, s32 * cmd,int len,int lock)

There is only one place that calls dbri_cmdsend(lock=0), namely
xmit_fixed().  Hence, I suggest not to add this argument but just add
spin_lock_irqsave()/spin_unlock_irqrestore() around the calling side
in xmit_fixed().  The spinlock per argument is hard to trace whether
the lock is really needed or not.

Otherwise the patch looks fine.

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] 4+ messages in thread

* Re: sparc dbri: SMP fixes
  2006-09-01 10:30 ` Takashi Iwai
@ 2006-09-05 18:14   ` Krzysztof Helt
  2006-09-05 18:22     ` Takashi Iwai
  0 siblings, 1 reply; 4+ messages in thread
From: Krzysztof Helt @ 2006-09-05 18:14 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel

[-- Attachment #1: Type: text/plain, Size: 628 bytes --]

Dnia 1-09-2006 o godz. 12:30 Takashi Iwai napisał(a):
> 
> There is only one place that calls dbri_cmdsend(lock=0), namely
> xmit_fixed().  Hence, I suggest not to add this argument but
just add
> spin_lock_irqsave()/spin_unlock_irqrestore() around the calling
side
> in xmit_fixed().  The spinlock per argument is hard to trace
whether
> the lock is really needed or not.
> 

This is a corrected patch.

Regards,
Krzysztof

----------------------------------------------------
Dzieci.wp.pl - Wydrukuj sobie plan lekcji - kliknij:
http://klik.wp.pl/?adr=http%3A%2F%2Fadv.reklama.wp.pl%2Fas%2Fd1.html&sid=860

[-- Attachment #2: dbri-patch15.diff --]
[-- Type: application/octet-stream, Size: 9140 bytes --]

--- alsa-driver-1.0.12rc2/alsa-kernel/sparc/dbri.c	2006-09-04 19:30:28.000000000 +0200
+++ linux-2.6.17a/sound/sparc/dbri.c	2006-09-04 19:36:45.000000000 +0200
@@ -635,10 +635,16 @@ to send them to the DBRI.
 static void dbri_cmdwait(struct snd_dbri *dbri)
 {
 	int maxloops = MAXLOOPS;
+	unsigned long flags;
 
 	/* Delay if previous commands are still being processed */
-	while ((--maxloops) > 0 && (sbus_readl(dbri->regs + REG0) & D_P))
+	spin_lock_irqsave(&dbri->lock, flags);
+	while ((--maxloops) > 0 && (sbus_readl(dbri->regs + REG0) & D_P)) {
+		spin_unlock_irqrestore(&dbri->lock, flags);
 		msleep_interruptible(1);
+		spin_lock_irqsave(&dbri->lock, flags);
+	}
+	spin_unlock_irqrestore(&dbri->lock, flags);
 
 	if (maxloops == 0) {
 		printk(KERN_ERR "DBRI: Chip never completed command buffer\n");
@@ -671,11 +677,12 @@ static s32 *dbri_cmdlock(struct snd_dbri
  * the last WAIT cmd and force DBRI to reread the cmd.
  * The JUMP cmd points to the new cmd string.
  * It also releases the cmdlock spinlock.
+ *
+ * Lock must not be held before calling this.
  */
 static void dbri_cmdsend(struct snd_dbri * dbri, s32 * cmd,int len)
 {
 	s32 tmp, addr;
-	unsigned long flags;
 	static int wait_id = 0;
 
 	wait_id++;
@@ -706,12 +713,10 @@ static void dbri_cmdsend(struct snd_dbri
 	}
 #endif
 
-	spin_lock_irqsave(&dbri->lock, flags);
 	/* Reread the last command */
 	tmp = sbus_readl(dbri->regs + REG0);
 	tmp |= D_P;
 	sbus_writel(tmp, dbri->regs + REG0);
-	spin_unlock_irqrestore(&dbri->lock, flags);
 
 	dbri->cmdptr = cmd;
 	spin_unlock(&dbri->cmdlock);
@@ -777,9 +782,9 @@ static void dbri_initialize(struct snd_d
 	dma_addr = dbri->dma_dvma + dbri_dma_off(cmd, 0);
 	sbus_writel(dma_addr, dbri->regs + REG8);
 	spin_unlock(&dbri->cmdlock);
-	dbri_cmdwait(dbri);
 
 	spin_unlock_irqrestore(&dbri->lock, flags);
+	dbri_cmdwait(dbri);
 }
 
 /*
@@ -840,6 +845,9 @@ static void reset_pipe(struct snd_dbri *
 	dbri->pipes[pipe].first_desc = -1;
 }
 
+/*
+ * Lock must be held before calling this.
+ */
 static void setup_pipe(struct snd_dbri * dbri, int pipe, int sdp)
 {
 	if (pipe < 0 || pipe > DBRI_MAX_PIPE) {
@@ -866,6 +874,9 @@ static void setup_pipe(struct snd_dbri *
 	reset_pipe(dbri, pipe);
 }
 
+/*
+ * Lock must be held before calling this.
+ */
 static void link_time_slot(struct snd_dbri * dbri, int pipe,
 			   int prevpipe, int nextpipe,
 			   int length, int cycle)
@@ -920,6 +931,10 @@ static void link_time_slot(struct snd_db
 	dbri_cmdsend(dbri, cmd, 4);
 }
 
+#if 0
+/*
+ * Lock must be held before calling this.
+ */
 static void unlink_time_slot(struct snd_dbri * dbri, int pipe,
 			     enum in_or_out direction, int prevpipe,
 			     int nextpipe)
@@ -952,6 +967,7 @@ static void unlink_time_slot(struct snd_
 
 	dbri_cmdsend(dbri, cmd, 4);
 }
+#endif
 
 /* xmit_fixed() / recv_fixed()
  *
@@ -965,11 +981,14 @@ static void unlink_time_slot(struct snd_
  * the actual time slot is.  The interrupt handler takes care of bit
  * ordering and alignment.  An 8-bit time slot will always end up
  * in the low-order 8 bits, filled either MSB-first or LSB-first,
- * depending on the settings passed to setup_pipe()
+ * depending on the settings passed to setup_pipe().
+ *
+ * Lock must not be held before calling it.
  */
 static void xmit_fixed(struct snd_dbri * dbri, int pipe, unsigned int data)
 {
 	s32 *cmd;
+	unsigned long flags;
 
 	if (pipe < 16 || pipe > DBRI_MAX_PIPE) {
 		printk(KERN_ERR "DBRI: xmit_fixed: Illegal pipe number\n");
@@ -1002,8 +1021,11 @@ static void xmit_fixed(struct snd_dbri *
 	*(cmd++) = data;
 	*(cmd++) = DBRI_CMD(D_PAUSE, 0, 0);
 
+	spin_lock_irqsave(&dbri->lock, flags);
 	dbri_cmdsend(dbri, cmd, 3);
+	spin_unlock_irqrestore(&dbri->lock, flags);
 	dbri_cmdwait(dbri);
+
 }
 
 static void recv_fixed(struct snd_dbri * dbri, int pipe, volatile __u32 * ptr)
@@ -1039,6 +1061,8 @@ static void recv_fixed(struct snd_dbri *
  * be spread across multiple descriptors.
  *
  * All descriptors create a ring buffer.
+ *
+ * Lock must be held before calling this.
  */
 static int setup_descs(struct snd_dbri * dbri, int streamno, unsigned int period)
 {
@@ -1186,6 +1210,9 @@ multiplexed serial interface which the D
 
 enum master_or_slave { CHImaster, CHIslave };
 
+/*
+ * Lock must not be held before calling it.
+ */
 static void reset_chi(struct snd_dbri * dbri, enum master_or_slave master_or_slave,
 		      int bits_per_frame)
 {
@@ -1258,9 +1285,14 @@ static void reset_chi(struct snd_dbri * 
 In the standard SPARC audio configuration, the CS4215 codec is attached
 to the DBRI via the CHI interface and few of the DBRI's PIO pins.
 
+ * Lock must not be held before calling it.
+
 */
 static void cs4215_setup_pipes(struct snd_dbri * dbri)
 {
+	unsigned long flags;
+
+	spin_lock_irqsave(&dbri->lock, flags);
 	/*
 	 * Data mode:
 	 * Pipe  4: Send timeslots 1-4 (audio data)
@@ -1284,6 +1316,7 @@ static void cs4215_setup_pipes(struct sn
 	setup_pipe(dbri, 17, D_SDP_FIXED | D_SDP_TO_SER | D_SDP_MSB);
 	setup_pipe(dbri, 18, D_SDP_FIXED | D_SDP_FROM_SER | D_SDP_MSB);
 	setup_pipe(dbri, 19, D_SDP_FIXED | D_SDP_FROM_SER | D_SDP_MSB);
+	spin_unlock_irqrestore(&dbri->lock, flags);
 
 	dbri_cmdwait(dbri);
 }
@@ -1358,6 +1391,7 @@ static void cs4215_open(struct snd_dbri 
 {
 	int data_width;
 	u32 tmp;
+	unsigned long flags;
 
 	dprintk(D_MM, "cs4215_open: %d channels, %d bits\n",
 		dbri->mm.channels, dbri->mm.precision);
@@ -1382,6 +1416,7 @@ static void cs4215_open(struct snd_dbri 
 	 * bits.  The CS4215, it seems, observes TSIN (the delayed signal)
 	 * even if it's the CHI master.  Don't ask me...
 	 */
+	spin_lock_irqsave(&dbri->lock, flags);
 	tmp = sbus_readl(dbri->regs + REG0);
 	tmp &= ~(D_C);		/* Disable CHI */
 	sbus_writel(tmp, dbri->regs + REG0);
@@ -1409,6 +1444,7 @@ static void cs4215_open(struct snd_dbri 
 	tmp = sbus_readl(dbri->regs + REG0);
 	tmp |= D_C;		/* Enable CHI */
 	sbus_writel(tmp, dbri->regs + REG0);
+	spin_unlock_irqrestore(&dbri->lock, flags);
 
 	cs4215_setdata(dbri, 0);
 }
@@ -1420,6 +1456,7 @@ static int cs4215_setctrl(struct snd_dbr
 {
 	int i, val;
 	u32 tmp;
+	unsigned long flags;
 
 	/* FIXME - let the CPU do something useful during these delays */
 
@@ -1456,6 +1493,7 @@ static int cs4215_setctrl(struct snd_dbr
 	 * done in hardware by a TI 248 that delays the DBRI->4215
 	 * frame sync signal by eight clock cycles.  Anybody know why?
 	 */
+	spin_lock_irqsave(&dbri->lock, flags);
 	tmp = sbus_readl(dbri->regs + REG0);
 	tmp &= ~D_C;		/* Disable CHI */
 	sbus_writel(tmp, dbri->regs + REG0);
@@ -1472,14 +1510,17 @@ static int cs4215_setctrl(struct snd_dbr
 	link_time_slot(dbri, 17, 16, 16, 32, dbri->mm.offset);
 	link_time_slot(dbri, 18, 16, 16, 8, dbri->mm.offset);
 	link_time_slot(dbri, 19, 18, 16, 8, dbri->mm.offset + 48);
+	spin_unlock_irqrestore(&dbri->lock, flags);
 
 	/* Wait for the chip to echo back CLB (Control Latch Bit) as zero */
 	dbri->mm.ctrl[0] &= ~CS4215_CLB;
 	xmit_fixed(dbri, 17, *(int *)dbri->mm.ctrl);
 
+	spin_lock_irqsave(&dbri->lock, flags);
 	tmp = sbus_readl(dbri->regs + REG0);
 	tmp |= D_C;		/* Enable CHI */
 	sbus_writel(tmp, dbri->regs + REG0);
+	spin_unlock_irqrestore(&dbri->lock, flags);
 
 	for (i = 10; ((dbri->mm.status & 0xe4) != 0x20); --i) {
 		msleep_interruptible(1);
@@ -1688,6 +1729,7 @@ static void xmit_descs(struct snd_dbri *
 			dbri->pipes[info->pipe].desc = first_td;
 		}
 	}
+
 	spin_unlock_irqrestore(&dbri->lock, flags);
 }
 
@@ -2093,7 +2135,6 @@ static int snd_dbri_prepare(struct snd_p
 {
 	struct snd_dbri *dbri = snd_pcm_substream_chip(substream);
 	struct dbri_streaminfo *info = DBRI_STREAM(dbri, substream);
-	struct snd_pcm_runtime *runtime = substream->runtime;
 	int ret;
 
 	info->size = snd_pcm_lib_buffer_bytes(substream);
@@ -2232,7 +2273,6 @@ static int snd_cs4215_put_volume(struct 
 {
 	struct snd_dbri *dbri = snd_kcontrol_chip(kcontrol);
 	struct dbri_streaminfo *info = &dbri->stream_info[kcontrol->private_value];
-	unsigned long flags;
 	int changed = 0;
 
 	if (info->left_gain != ucontrol->value.integer.value[0]) {
@@ -2247,13 +2287,9 @@ static int snd_cs4215_put_volume(struct 
 		/* First mute outputs, and wait 1/8000 sec (125 us)
 		 * to make sure this takes.  This avoids clicking noises.
 		 */
-		spin_lock_irqsave(&dbri->lock, flags);
-
 		cs4215_setdata(dbri, 1);
 		udelay(125);
 		cs4215_setdata(dbri, 0);
-
-		spin_unlock_irqrestore(&dbri->lock, flags);
 	}
 	return changed;
 }
@@ -2300,7 +2336,6 @@ static int snd_cs4215_put_single(struct 
 				 struct snd_ctl_elem_value *ucontrol)
 {
 	struct snd_dbri *dbri = snd_kcontrol_chip(kcontrol);
-	unsigned long flags;
 	int elem = kcontrol->private_value & 0xff;
 	int shift = (kcontrol->private_value >> 8) & 0xff;
 	int mask = (kcontrol->private_value >> 16) & 0xff;
@@ -2333,13 +2368,9 @@ static int snd_cs4215_put_single(struct 
 		/* First mute outputs, and wait 1/8000 sec (125 us)
 		 * to make sure this takes.  This avoids clicking noises.
 		 */
-		spin_lock_irqsave(&dbri->lock, flags);
-
 		cs4215_setdata(dbri, 1);
 		udelay(125);
 		cs4215_setdata(dbri, 0);
-
-		spin_unlock_irqrestore(&dbri->lock, flags);
 	}
 	return changed;
 }

[-- Attachment #3: Type: text/plain, Size: 373 bytes --]

-------------------------------------------------------------------------
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

[-- Attachment #4: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: sparc dbri: SMP fixes
  2006-09-05 18:14   ` Krzysztof Helt
@ 2006-09-05 18:22     ` Takashi Iwai
  0 siblings, 0 replies; 4+ messages in thread
From: Takashi Iwai @ 2006-09-05 18:22 UTC (permalink / raw)
  To: Krzysztof Helt; +Cc: alsa-devel

At Tue, 05 Sep 2006 20:14:11 +0200,
Krzysztof Helt wrote:
> 
> Dnia 1-09-2006 o godz. 12:30 Takashi Iwai napisał(a):
> > 
> > There is only one place that calls dbri_cmdsend(lock=0), namely
> > xmit_fixed().  Hence, I suggest not to add this argument but
> just add
> > spin_lock_irqsave()/spin_unlock_irqrestore() around the calling
> side
> > in xmit_fixed().  The spinlock per argument is hard to trace
> whether
> > the lock is really needed or not.
> > 
> 
> This is a corrected patch.

Thanks, applied to HG tree now.


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] 4+ messages in thread

end of thread, other threads:[~2006-09-05 18:22 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-09-01  7:11 sparc dbri: SMP fixes Krzysztof Helt
2006-09-01 10:30 ` Takashi Iwai
2006-09-05 18:14   ` Krzysztof Helt
2006-09-05 18:22     ` Takashi Iwai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.