[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: problem with sound driver in iBook2



On Thu, 2003-07-31 at 23:34, Heinz Kirchmann wrote: 
> I tried to collect some info, why the dmasound driver does not work 
> correctly on my iBook2. Besides the fact, that recording is not possible 
> with the standard dmasound_pmac and dmasound_core drivers, I noticed 
> problems with audacity, too. After calling audacity (version 1.0.1) 
> neither recording nor playing were possible any more. I made some tests 
> and it turned out, that opening /dev/dsp with flag O_RDWR caused the 
> driver not to release the device cleanly.
> 
> System:
>    Hardware: iBook2 G3 800 12"
>    OS: Debian 3.0
>    Kernel: 2.4.21-ben2 (from rsync, no configuration changes)
> 
> I took a closer look at the kernel sources of dmasound_core.c and 
> dmasound_awacs.c in subdirectory 'drivers/sound/dmasound'.
> 
> I noticed the following strange things in dmasound_core.c and 
> dmasound_awacs.c:
> 
> a) in sq_open (dmasound_core.c) I found two (conflicting?) concepts to 
> decide, if the soundcard offers recording possibility or not: macro 
> 'HAS_RECORD' was set and dmasound.mach.record was _not_ set. 

They aren't conflicting. HAS_RECORD defines whether the code for
recording is built in at all, dmasound.mach.record says whether
recording is supported for the machine it's running on.

> I decided to change the corresponding line in function dmasound_awacs_init 
> (file dmasound_awacs.c) around line 3109:
> 
>          if (awacs_revision != AWACS_TUMBLER &&
>              awacs_revision != AWACS_SNAPPER &&
>              awacs_revision != AWACS_DACA)
> 		dmasound.mach.capabilities = DSP_CAP_DUPLEX ;
> 		dmasound.mach.record = PMacRecord ;
> ...
> 
> I commented out the line with AWACS_SNAPPER in it, thus causing 
> dmasound.mach.record to be set to a value != NULL.
> 
> b) the structure sq_fops of type 'struct file_operations' (in 
> dmasound_core.c) does not define a function for 'read'. Even though 
> 'HAS_RECORD' is defined, the corresponding value is set to NULL. What am 
> I missing here?
> I decided to set the value for 'read' to 'sq_read'.

This is done in sq_init() if recording is supported.

> These two changes changed the behaviour of the driver as follows:
> a) open for reading now possible, but a call to read will cause the read 
> function to return immediately with successful return value but returned 
> data does not contain any senseful samples.
> b) driver does not lock up sound device after opening with O_RDWR.

Well, this just avoids the problem by pretending that recording is
supported, when in fact it isn't.

However, as some apps open the device with O_RDWR even when they're
never going to read from it, this might not be the worst approach. This
patch does more or less what you describe, but reads will return
-EINVAL.

Does anybody see a problem with this? (I mean a more annoying problem
than the one it works around? :)


-- 
Earthling Michel Dänzer   \  Debian (powerpc), XFree86 and DRI developer
Software libre enthusiast  \     http://svcs.affero.net/rm.php?r=daenzer
diff -up -r linux-2.4.20-ben8/drivers/sound/dmasound/dmasound_core.c linux-2.4.20-ben8-xfs-lolat/drivers/sound/dmasound/dmasound_core.c
--- linux-2.4.20-ben8/drivers/sound/dmasound/dmasound_core.c	2002-07-26 14:11:41.000000000 +0200
+++ linux-2.4.20-ben8-xfs-lolat/drivers/sound/dmasound/dmasound_core.c	2003-08-01 13:28:54.000000000 +0200
@@ -881,13 +881,6 @@ static int sq_open(struct inode *inode, 
 			dmasound.mach.release();
 			return rc;
 		}
-	} else { /* no record function installed; in compat mode */
-		if (file->f_mode & FMODE_READ) {
-			/* TODO: if O_RDWR, release any resources grabbed by write part */
-			dmasound.mach.release() ;
-			/* I think this is what is required by open(2) */
-			return -ENXIO ;
-		}
 	}
 #else /* !HAS_RECORD */
 	if (file->f_mode & FMODE_READ) {
@@ -1333,7 +1326,7 @@ static struct file_operations sq_fops =
 	open:		sq_open,
 	release:	sq_release,
 #ifdef HAS_RECORD
-	read:		NULL	/* default to no read for compat mode */
+	read:		sq_read,
 #endif
 };
 
@@ -1343,10 +1336,6 @@ static int __init sq_init(void)
 	int sq_unit;
 #endif
 
-#ifdef HAS_RECORD
-	if (dmasound.mach.record)
-		sq_fops.read = sq_read ;
-#endif
 	sq_unit = register_sound_dsp(&sq_fops, -1);
 	if (sq_unit < 0) {
 		printk(KERN_ERR "dmasound_core: couldn't register fops\n") ;

Reply to: