re.Fix hang in mpu401_uart.c patch

re.Fix hang in mpu401_uart.c patch

[Alan Horstmann]

This patch since 1.0.11 throws up an underlying problem with au88x0 (separate 
posting refers), but I also noticed that the 4th hunk applys changes to 
snd_mpu401_uart_input_close and snd_mpu401_uart_output_close.

However, as the 3rd augument is '0' ie don't check ACK, OK is always true in 
snd_mpu401_uart_cmd and there is surely no point to the extra check lines?

(As my separate posting 'au88x0_mpu401 init / fail first open' mentions, ACK 
is not returned from the RESET during the close, either with ice1712 or 
au8820, so can't be checked for.)

Alan



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

Re: re.Fix hang in mpu401_uart.c patch

[Takashi Iwai]

At Wed, 10 May 2006 16:10:41 +0100,
Alan Horstmann wrote:
> 
> This patch since 1.0.11 throws up an underlying problem with au88x0 (separate 
> posting refers), but I also noticed that the 4th hunk applys changes to 
> snd_mpu401_uart_input_close and snd_mpu401_uart_output_close.
> 
> However, as the 3rd augument is '0' ie don't check ACK, OK is always true in 
> snd_mpu401_uart_cmd and there is surely no point to the extra check lines?
> 
> (As my separate posting 'au88x0_mpu401 init / fail first open' mentions, ACK 
> is not returned from the RESET during the close, either with ice1712 or 
> au8820, so can't be checked for.)

Which patch are you referring to??


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

Re: re.Fix hang in mpu401_uart.c patch

[Alan Horstmann]

On Thursday 11 May 2006 15:07, you wrote:
> At Wed, 10 May 2006 16:10:41 +0100,
>
> Alan Horstmann wrote:
> > This patch since 1.0.11 throws up an underlying problem with au88x0
> > (separate posting refers), but I also noticed that the 4th hunk applys
> > changes to snd_mpu401_uart_input_close and snd_mpu401_uart_output_close.
> >
> > However, as the 3rd augument is '0' ie don't check ACK, OK is always true
> > in snd_mpu401_uart_cmd and there is surely no point to the extra check
> > lines?
> >
> > (As my separate posting 'au88x0_mpu401 init / fail first open' mentions,
> > ACK is not returned from the RESET during the close, either with ice1712
> > or au8820, so can't be checked for.)
>
> Which patch are you referring to??

The one titled  'Fix hang in mpu401_uart.c',  on repo about 2 weeks ago, by 
Jon Masters.

Alan



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

Re: re.Fix hang in mpu401_uart.c patch

[Alan Horstmann]

On Saturday 13 May 2006 13:04, I wrote:
> On Thursday 11 May 2006 15:07, you wrote:
> > At Wed, 10 May 2006 16:10:41 +0100,
> >
> > Alan Horstmann wrote:
> > > This patch since 1.0.11 throws up an underlying problem with au88x0
> > > (separate posting refers), but I also noticed that the 4th hunk applys
> > > changes to snd_mpu401_uart_input_close and
> > > snd_mpu401_uart_output_close.
> > >
> > > However, as the 3rd augument is '0' ie don't check ACK, OK is always
> > > true in snd_mpu401_uart_cmd and there is surely no point to the extra
> > > check lines?
> > >
> > > (As my separate posting 'au88x0_mpu401 init / fail first open'
> > > mentions, ACK is not returned from the RESET during the close, either
> > > with ice1712 or au8820, so can't be checked for.)
> >
> > Which patch are you referring to??
>
> The one titled  'Fix hang in mpu401_uart.c',  on repo about 2 weeks ago, by
> Jon Masters.

Specifically, chunks below:-

Alan

 static int snd_mpu401_uart_input_close(struct snd_rawmidi_substream 
*substream)
 {
 	struct snd_mpu401 *mpu;
+	int err = 0;

 	mpu = substream->rmidi->private_data;
 	clear_bit(MPU401_MODE_BIT_INPUT, &mpu->mode);
 	mpu->substream_input = NULL;
 	if (! test_bit(MPU401_MODE_BIT_OUTPUT, &mpu->mode))
-		snd_mpu401_uart_cmd(mpu, MPU401_RESET, 0);
+		err = snd_mpu401_uart_cmd(mpu, MPU401_RESET, 0);
 	if (mpu->close_input)
 		mpu->close_input(mpu);
+	if (err)
+		return -EIO;
 	return 0;
 }

 static int snd_mpu401_uart_output_close(struct snd_rawmidi_substream 
*substream)
 {
 	struct snd_mpu401 *mpu;
+	int err = 0;

 	mpu = substream->rmidi->private_data;
 	clear_bit(MPU401_MODE_BIT_OUTPUT, &mpu->mode);
 	mpu->substream_output = NULL;
 	if (! test_bit(MPU401_MODE_BIT_INPUT, &mpu->mode))
-		snd_mpu401_uart_cmd(mpu, MPU401_RESET, 0);
+		err = snd_mpu401_uart_cmd(mpu, MPU401_RESET, 0);
 	if (mpu->close_output)
 		mpu->close_output(mpu);
+	if (err)
+		return -EIO;
 	return 0;
 }

@@ -316,6 +339,7 @@ static void snd_mpu401_uart_input_trigge
 			snd_mpu401_uart_remove_timer(mpu, 1);
 		clear_bit(MPU401_MODE_BIT_INPUT_TRIGGER, &mpu->mode);
 	}
+
 }

 /*




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

Re: re.Fix hang in mpu401_uart.c patch

[Jon Masters]

Folks,

I'm not on ALSA devel. Care you enlighten me as to what the problem is?

I produced the original patch because without it "cat /dev/sequencer"
will kill my machine when my sound driver tries to talk to an MPU that
doesn't exist.

Jon.


-------------------------------------------------------
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\x120709&bid&3057&dat\x121642

Re: re.Fix hang in mpu401_uart.c patch

[Jon Masters]

Ok. I signed up to alsa-devel and read the thread...

On 5/15/06, Alan Horstmann <gineera@aspect135.co.uk> wrote:

> > > > However, as the 3rd augument is '0' ie don't check ACK, OK is always
> > > > true in snd_mpu401_uart_cmd and there is surely no point to the extra
> > > > check lines?

IIRC I added the extra checks you refer to for completeness (it's bad
form not to check return), but the error path in snd_mpu401_uart_cmd
is getting called on my system when it's called during init -
printk()ing a warning is useless, the function should return an error.

What's the problem anyway that you're experiencing?

Jon.


-------------------------------------------------------
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\x120709&bid&3057&dat\x121642

Re: re.Fix hang in mpu401_uart.c patch

[Alan Horstmann]

On Tuesday 16 May 2006 18:50, you wrote:
> Ok. I signed up to alsa-devel and read the thread...
>
> On 5/15/06, Alan Horstmann <gineera@aspect135.co.uk> wrote:
> > > > > However, as the 3rd augument is '0' ie don't check ACK, OK is
> > > > > always true in snd_mpu401_uart_cmd and there is surely no point to
> > > > > the extra check lines?
>
> IIRC I added the extra checks you refer to for completeness (it's bad
> form not to check return), but the error path in snd_mpu401_uart_cmd
> is getting called on my system when it's called during init -
> printk()ing a warning is useless, the function should return an error.
>
> What's the problem anyway that you're experiencing?

It's no big deal -it's not causing any problems, and I understand entirely why 
the function should be able to return error, and that the return should be 
checked.  However I just noticed that in the case of the 2 'close' functions, 
calling snd_mpu401_uart_cmd with 3rd arg '0', an error could never be 
returned (ok always=1), and so there is no point in checking for it.  As 
Takashi is normally keen to omit any unnecessary lines, I thought I would 
mention it.

Alan



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

Re: re.Fix hang in mpu401_uart.c patch

[Takashi Iwai]

At Wed, 17 May 2006 22:45:21 +0100,
Alan Horstmann wrote:
> 
> On Tuesday 16 May 2006 18:50, you wrote:
> > Ok. I signed up to alsa-devel and read the thread...
> >
> > On 5/15/06, Alan Horstmann <gineera@aspect135.co.uk> wrote:
> > > > > > However, as the 3rd augument is '0' ie don't check ACK, OK is
> > > > > > always true in snd_mpu401_uart_cmd and there is surely no point to
> > > > > > the extra check lines?
> >
> > IIRC I added the extra checks you refer to for completeness (it's bad
> > form not to check return), but the error path in snd_mpu401_uart_cmd
> > is getting called on my system when it's called during init -
> > printk()ing a warning is useless, the function should return an error.
> >
> > What's the problem anyway that you're experiencing?
> 
> It's no big deal -it's not causing any problems, and I understand entirely why 
> the function should be able to return error, and that the return should be 
> checked.  However I just noticed that in the case of the 2 'close' functions, 
> calling snd_mpu401_uart_cmd with 3rd arg '0', an error could never be 
> returned (ok always=1), and so there is no point in checking for it.  As 
> Takashi is normally keen to omit any unnecessary lines, I thought I would 
> mention it.

Yeah it's a small dead code right now.

But in this case it's better to keep the error check for possible
future changes (for example, snd_mpu401_uart_cmd() returns an error
when tx timeout occurs).  If we remove the error check there, this
change would have no effect, too.


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