[parisc-linux] [PATCH] Harmony driver - Reject AFMT_S16_LE & Implement SNDCTL_DSP_CHANNELS

[Stuart Brady <sdbrady@ntlworld.com>]

On Sun, Feb 22, 2004 at 09:20:57AM +0000, Stuart Brady wrote:

> +				if (put_user(ival, (int *) arg))
> +					return -EFAULT;

I've been looking at the specification, I think this should have been:

return put_user(ival, (int *) arg));

I assumed that -EINVAL would be returned, but the example shows the
argument being changed, without the ioctl returning an error. I'm still
not sure whether "harmony_set_format(HARMONY_DF_16BIT_LINEAR)" is needed
when an unsupported format is requested.  The specification is not clear
to me. "Rejects" implies "does not set the format" to me.

I'm going to have to get a firm idea of what each driver currently does
(since they tend to do different things anyway), what the apps want, and
what the documentation states.

I've also added a SNDCTL_DSP_CHANNELS (needed for mikmod). Here is the
revised patch:

Index: harmony.c
===================================================================
RCS file: /var/cvs/linux-2.4/drivers/sound/harmony.c,v
retrieving revision 1.28
diff -u -r1.28 harmony.c
--- harmony.c	22 Jun 2002 09:05:59 -0000	1.28
+++ harmony.c	23 Feb 2004 03:28:38 -0000
@@ -641,18 +641,17 @@
 			switch (ival) {
 			case AFMT_MU_LAW:	new_format = HARMONY_DF_8BIT_ULAW; break;
 			case AFMT_A_LAW:	new_format = HARMONY_DF_8BIT_ALAW; break;
-			case AFMT_S16_LE:	/* fall through, but not really supported */
-			case AFMT_S16_BE:	new_format = HARMONY_DF_16BIT_LINEAR;
-						ival = AFMT_S16_BE;
-						break; 
+			case AFMT_S16_BE:	new_format = HARMONY_DF_16BIT_LINEAR; break;
 			default: {
 				DPRINTK(KERN_WARNING PFX 
 					"unsupported sound format 0x%04x requested.\n",
 					ival);
-				return -EINVAL;
+				ival = AFMT_S16_BE;
+				return put_user(ival, (int *) arg);
 			}
 			}
 			harmony_set_format(new_format);
+			return 0;
 		} else {
 			switch (harmony.data_format) {
 			case HARMONY_DF_8BIT_ULAW:	ival = AFMT_MU_LAW; break;
@@ -660,8 +659,8 @@
 			case HARMONY_DF_16BIT_LINEAR:	ival = AFMT_U16_BE; break;
 			default: ival = 0;
 			}
+			return put_user(ival, (int *) arg);
 		}
-		return put_user(ival, (int *) arg);
 
 	case SOUND_PCM_READ_RATE:
 		ival = harmony.dac_rate;
@@ -680,7 +679,17 @@
 		if (ival != 0 && ival != 1)
 			return -EINVAL;
 		harmony_set_stereo(ival);
-		return put_user(ival, (int *) arg);
+		return 0;
+
+	case SNDCTL_DSP_CHANNELS:
+		if (get_user(ival, (int *) arg))
+			return -EFAULT;
+		if (ival != 1 && ival != 2) {
+			ival = harmony.stereo_select == HARMONY_SS_MONO ? 1 : 2;
+			return put_user(ival, (int *) arg);
+		}
+		harmony_set_stereo(ival-1);
+		return 0;
 
 	case SNDCTL_DSP_GETBLKSIZE:
 		ival = HARMONY_BUF_SIZE;

Mikmod now plays audio, but it says "Loading" at the top and you can't
do anything in the UI. I'll look into this after I've fixed the mixer.
The internal speaker, headphones, and rear jack don't seem to fit well
with the existing mixer channels in OSS. Any recommendations?

I'm not sure how SOUND_PCM_WRITE_CHANNELS and SOUND_PCM_READ_CHANNELS
are handled. Looks like they're the same ioctl. How does that work?!
-- 
Stuart Brady