[parisc-linux] [PATCH] Harmony driver - Reject AFMT_S16_LE (Little Endian)

[parisc-linux] [PATCH] Harmony driver - Reject AFMT_S16_LE (Little Endian)

[Stuart Brady]

Hello,

The OSS specification (http://www.opensound.com/pguide/oss.pdf -
page 33) states:

"It is very important to check that the value returned in the argument
after the [SNDCTL_DSP_SETFMT] ioctl call matches the requested format.
If the device doesn't support this particular format, it rejects the
call and returns another format that is supported by the hardware."

I would suggest the following change to the Harmony driver:

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	22 Feb 2004 05:35:46 -0000
@@ -641,14 +641,14 @@
 			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);
+				ival = AFMT_S16_BE;
+				if (put_user(ival, (int *) arg))
+					return -EFAULT;
 				return -EINVAL;
 			}
 			}

Since mu-law and a-law aren't the most popular of formats, I think
AFMT_S16_BE (signed, 16-bit, big endian) is the best thing to return.
Some drivers don't seem to return a supported format in arg - I'm not
sure why. At the very least, I think -EINVAL should be returned, and the
format should not be set. (Unless of course, the driver actually _does_
the conversion - okay if I implement this?)

This change may break some apps. The soundcard.h fix might make a
significant difference. Anything else was writing big endian data using
a little endian format.

One question: are there any audio devices for hppa that need drivers,
that have documentation? I see that audio on the J5k/C3k is unsupported,
but there are apparently no docs for it. Audio on the 705 and 710 doesn't
seem supported either. Again, no docs AFAICS.
-- 
Stuart Brady

Re: [parisc-linux] [PATCH] Harmony driver - Reject AFMT_S16_LE (Little Endian)

[Stuart Brady]

I said:

> One question: are there any audio devices for hppa that need drivers,
> that have documentation? I see that audio on the J5k/C3k is unsupported,
> but there are apparently no docs for it. Audio on the 705 and 710 doesn't
> seem supported either. Again, no docs AFAICS.

I do apologise - I've now noticed that there _is_ a AD1889 driver,
although I don't know whether it works on hppa.

http://lists.parisc-linux.org/pipermail/parisc-linux/2002-January/015138.html
looks quite useful - I'll have to see if I can get docs on the PSB2160.

It's interesting that the 720/730/750 has 3 channel beeper sound. Could
someone tell me the name of the chip, please?

http://members.chello.nl/h.otten/vortexion.htm has a list of popular
sound chips for microcomputers in the 80s. It'd be great if it turned
out to be an AY-3-8910, YM2149 or SN76489, as there's a lot of music
that's been written for those chips.
-- 
Stuart Brady

Re: [parisc-linux] ad1889 driver/docs for c3k/j5k audio

[Grant Grundler]

On Sun, Feb 22, 2004 at 09:20:57AM +0000, Stuart Brady wrote:
...
> One question: are there any audio devices for hppa that need drivers,
> that have documentation? I see that audio on the J5k/C3k is unsupported,

As you noted ad1889 is available but only sort-of works:
http://lists.parisc-linux.org/pipermail/parisc-linux/2003-April/019650.html

I posted the only documentation we have for ad1889:
http://lists.parisc-linux.org/pipermail/parisc-linux/2002-June/016704.html

If you have patches for ad1889, I'm happy to test (and commit) them for you.

thanks,
grant

[parisc-linux] Re: ad1889 driver/docs for c3k/j5k audio

[Stuart Brady]

On Sun, Feb 22, 2004 at 09:25:44AM -0700, Grant Grundler wrote:

> I posted the only documentation we have for ad1889:
> http://lists.parisc-linux.org/pipermail/parisc-linux/2002-June/016704.html

> If you have patches for ad1889, I'm happy to test (and commit) them for you.

Unfortunately, the documentation you're referring to is no longer there.
Do you still have it?

If so, I don't have a box with an ad1889, but I should be able to make
some small improvements.
-- 
Stuart Brady
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

Re: [parisc-linux] Re: ad1889 driver/docs for c3k/j5k audio

[Randolph Chung]

> Unfortunately, the documentation you're referring to is no longer there.
> Do you still have it?

try: ftp://ftp.parisc-linux.org/docs/AD1889.pdf

> If so, I don't have a box with an ad1889, but I should be able to make
> some small improvements.

I think Mort is also looking at this.....

thanks :)
randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

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

[Stuart Brady]

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

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

[Grant Grundler]

On Mon, Feb 23, 2004 at 03:56:01AM +0000, Stuart Brady wrote:
> 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:
...

Hi Stuart,
Sorry for taking so long with this...I was expecting someone else
to reply and maybe missed it. A couple of minor problems with
your patch:

1) it's for 2.4 and I'm personally trying to focus on 2.6 tree now.
   2.6 is ready for prime time. It's been my desktop/ftp/http server
   (C3600) for the past couple of monthes and has proven very stable.

2) OSS is "decprecated" according to the 2.6 sound/Kconfig "Help".
   Ie use ALSA instead.  See SOUND_PRIME option.

3) if no one else wants to poke at the 2.4 harmony driver, I will
   just apply your patches on good faith that it builds/works for you.
   Have you tweaked it more since Feb?

In 2.6, ALSA harmony driver does NOT compile and that's disappointing.

thanks,
grant
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

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

[Matthew Wilcox]

On Mon, May 10, 2004 at 11:24:51PM -0600, Grant Grundler wrote:
> 2) OSS is "decprecated" according to the 2.6 sound/Kconfig "Help".
>    Ie use ALSA instead.  See SOUND_PRIME option.
> 
> In 2.6, ALSA harmony driver does NOT compile and that's disappointing.

The basic problem here is that ALSA doesn't use the generic device model.
It has its own crazy set of bus abstractions which would all need to
be implemented for the parisc_device ... for the benefit of one driver.
Small wonder nobody's been interested in doing the work yet.

I think we'll be sticking with OSS Harmony for a while.

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

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

[Paul]

Actually, ALSA Harmony driver is compiling at this point (2.6.6-rc3-pa9), 
with the caveat that you need the latest CVS of sound/parisc..
Takashi Iwai has been kind enough to work on this, with me doing some basic 
testing. Unfortunately, my testing hit a snag when a hard drive started 
releasing magic smoke ever so slowly.

Thanks
Paul

----- Original Message ----- 
From: "Matthew Wilcox" <willy@debian.org>
To: "Grant Grundler" <grundler@parisc-linux.org>
Cc: <parisc-linux@lists.parisc-linux.org>
Sent: Tuesday, May 11, 2004 8:07 AM
Subject: Re: [parisc-linux] [PATCH] Harmony driver - Reject AFMT_S16_LE 
&Implement SNDCTL_DSP_CHANNELS


> On Mon, May 10, 2004 at 11:24:51PM -0600, Grant Grundler wrote:
>> 2) OSS is "decprecated" according to the 2.6 sound/Kconfig "Help".
>>    Ie use ALSA instead.  See SOUND_PRIME option.
>>
>> In 2.6, ALSA harmony driver does NOT compile and that's disappointing.
>
> The basic problem here is that ALSA doesn't use the generic device model.
> It has its own crazy set of bus abstractions which would all need to
> be implemented for the parisc_device ... for the benefit of one driver.
> Small wonder nobody's been interested in doing the work yet.
>
> I think we'll be sticking with OSS Harmony for a while.
>
> -- 
> "Next the statesmen will invent cheap lies, putting the blame upon
> the nation that is attacked, and every man will be glad of those
> conscience-soothing falsities, and will diligently study them, and refuse
> to examine any refutations of them; and thus he will by and by convince
> himself that the war is just, and will thank God for the better sleep
> he enjoys after this process of grotesque self-deception." -- Mark Twain
> _______________________________________________
> parisc-linux mailing list
> parisc-linux@lists.parisc-linux.org
> http://lists.parisc-linux.org/mailman/listinfo/parisc-linux 

_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

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

[Stuart Brady]

Hi Grant,

> Sorry for taking so long with this...I was expecting someone else
> to reply and maybe missed it. A couple of minor problems with
> your patch:

Actually, I think I should have been more persistent.

> 1) it's for 2.4 and I'm personally trying to focus on 2.6 tree now.
>    2.6 is ready for prime time. It's been my desktop/ftp/http server
>    (C3600) for the past couple of monthes and has proven very stable.
> 
> 2) OSS is "decprecated" according to the 2.6 sound/Kconfig "Help".
>    Ie use ALSA instead.  See SOUND_PRIME option.
> 
> 3) if no one else wants to poke at the 2.4 harmony driver, I will
>    just apply your patches on good faith that it builds/works for you.
>    Have you tweaked it more since Feb?

I've made some improvements to the mixer code, but OSS's mixer interface
seems to be inadequate.  I can post a patch that improves the mixer code
slightly.  Other than that, I haven't changed anything.

Under high load, I tend to get panics when using the Harmony driver.
I've no idea why this is, and I wish I could understand the low-level
code a bit better.  This has nothing to do with my patches, though.
(I.e. it crashes with them and it crashes without them.)

http://lists.parisc-linux.org/pipermail/parisc-linux/2004-February/022432.html
is probably more of an issue for lkml.  (There's a slight mistake in
that patch - I forgot the closing */ on line 42.)

> In 2.6, ALSA harmony driver does NOT compile and that's disappointing.

I would love to try and help with the 2.6 drivers instead.
Unfortunatlely, I can't get 2.6 to boot.  This is all that I see:

Command line for kernel: 'root=/dev/sda6 HOME=/ console=tty0 sti=1
sti_font=VGA8x16 TERM=linux palo_kernel=2/vmlinux-2.6'
Selected kernel: /vmlinux-2.6 from partition 2
ELF32 executable
Entry 001001c0 first 00100000 n 3
Segment 0 load 00100000 size 2520296 mediaptr 0x1000
Segment 1 load 00368000 size 565504 mediaptr 0x269000
Segment 2 load 003f4000 size 581766 mediaptr 0x2f4000
Branching to kernel entry point 0x001001c0.  If this is the last
message you see, you may need to switch your console.  This is
a common symptom -- search the FAQ and mailing list at parisc-linux.org

I have another graphics card in this machine, so I'm wondering whether
that has anything to do with this.  FWIW, it's a 715/100.  I'm using the
prebuilt palinux-32-2.6.6-pa4_0-2_all.deb package from
http://cvs.parisc-linux.org/download/linux-2.6/

Thanks,
-- 
Stuart Brady
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

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

[Ruediger Scholz]

Hello,

Stuart Brady schrieb:

>>In 2.6, ALSA harmony driver does NOT compile and that's disappointing.
>>    
>>
as I (and others) wrote in a former mail ALSA harmony driver from ALSA 
cvs does compile. Just copy harmony.c from ALSA cvs into kernel source, 
fix two typos (haromny -> harmony) and voilà... Boot log shows:

    Advanced Linux Sound Architecture Driver Version 1.0.4.
    Lasi Harmony Audio driver h/w id 20, rev. 18 at 0xf0104000, IRQ 82
    harmony: Successfully registered harmony pcm backend & mixer 0
    ALSA device list:
      #0: ALSA driver for LASI Harmony at h/w, id 20, rev. 18 hpa
    0xf0104000, IRQ 82

There is just one problem ;). One can't unmute  main volume. I can 
change volume settings using aumix, but they get canceled when closing 
aumix. But perhaps this can be fixed easily by s.o. who has the force... ;)

>
>I would love to try and help with the 2.6 drivers instead.
>Unfortunatlely, I can't get 2.6 to boot.  This is all that I see:
>
>Command line for kernel: 'root=/dev/sda6 HOME=/ console=tty0 sti=1
>sti_font=VGA8x16 TERM=linux palo_kernel=2/vmlinux-2.6'
>Selected kernel: /vmlinux-2.6 from partition 2
>ELF32 executable
>Entry 001001c0 first 00100000 n 3
>Segment 0 load 00100000 size 2520296 mediaptr 0x1000
>Segment 1 load 00368000 size 565504 mediaptr 0x269000
>Segment 2 load 003f4000 size 581766 mediaptr 0x2f4000
>Branching to kernel entry point 0x001001c0.  If this is the last
>message you see, you may need to switch your console.  This is
>a common symptom -- search the FAQ and mailing list at parisc-linux.org
>  
>
I also have an 715/100 with two graphics card and I'm using current cvs 
kernel (2.6.7-rc1-pa1). It works for me. My command line for the kernel is:
'root=/dev/sdb4 HOME=/ console=tty0 sti=1 sti_font=VGA8x16 TERM=linux 
palo_kernel=2/vmlinux'. I can also give you my .config if you are 
interested in.

>I have another graphics card in this machine, so I'm wondering whether
>that has anything to do with this.  FWIW, it's a 715/100.  I'm using the
>prebuilt palinux-32-2.6.6-pa4_0-2_all.deb package from
>http://cvs.parisc-linux.org/download/linux-2.6/
>
>Thanks,
>  
>
Thanks,
Ruediger
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

[parisc-linux] [PATCH] Harmony - buffer underrun crash fix

[Stuart Brady]

On Mon, May 24, 2004 at 08:12:00AM +0200, Thibaut VARENE wrote:

> There's been a suspected buffer overflow in the code for years, and it's
> a known "feature" that the driver may randomly panic the kernel. Patch
> welcome.

Thanks.  I had assumed that this was black magic and I'd never be able
to fix it, but thanks to your encouragement, I've taken a look and I now
have a fix for the crash, as well as a sample skipping problem that I
was previously unaware of:

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	25 May 2004 19:15:21 -0000
@@ -537,6 +537,7 @@
 	int count = 0;
 	int frame_size;
 	int buf_to_fill;
+	int fresh_buffer;
 
 	if (!harmony.format_initialized) 
 	   harmony_format_auto_detect(buffer, total_count);
@@ -556,12 +557,16 @@
 		
 		
 		buf_to_fill = (harmony.first_filled_play+harmony.nb_filled_play); 
-		if (harmony.play_offset)
+		if (harmony.play_offset) {
 			buf_to_fill--;
+			buf_to_fill += MAX_BUFS;
+		}
 		buf_to_fill %= MAX_BUFS;
-
+		
+		fresh_buffer = (harmony.play_offset == 0);
+		
 		/* Figure out the size of the frame */
-		if ((total_count-count) > HARMONY_BUF_SIZE - harmony.play_offset) {
+		if ((total_count-count) >= HARMONY_BUF_SIZE - harmony.play_offset) {
 			frame_size = HARMONY_BUF_SIZE - harmony.play_offset;
 		} else {
 			frame_size = total_count - count;
@@ -578,7 +583,7 @@
 		CHECK_WBACK_INV_OFFSET(played_buf, (HARMONY_BUF_SIZE*buf_to_fill + harmony.play_offset), 
 				frame_size);
 	
-		if (!harmony.play_offset)
+		if (fresh_buffer)
 			harmony.nb_filled_play++;
 		
 		count += frame_size;

Lines 560-563 fix the crash, which was caused by buf_to_fill taking the
value -1 (since -1 % MAX_BUFS == -1).

Lines 540, 566, 586/587 should fix a bug where samples were skipped.

The change to line 569 prevents harmony_silence from being called
whenever the final frame that is to be copied completely fills a buffer.

I'm a bit of a newbie here... can a harmony_interrupt occur whilst we're
in harmony_audio_write?
-- 
Stuart Brady
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux