speakup bug

speakup bug

[John G. Heim]

I need help fixing a bug in the driver for serial hardware  speech synths in 
the speakup screen reader. According to the comments in the code, it is in a 
part of the code that is trying to "steal" the serial port.

First it calls request_region and when that fails (it always fails), it 
calls         __release_region(&then it calls release_region again to see if 
__release_region worked. But it never works because the region being 
requested is already taken.

The code in question is in 2 source code files in drivers/staging/speakup. 
It starts in serialio.c on line 38. Here it calls a function named 
synth_request_region which in turn, calls request_region. On line 41 it 
calls __release_region, and on line 42 it calls synth_request_region again. 
The function synth_request_region (which calls request_region) is in a file 
named synth.c. But this code always fails.  Here is a kind of simplified 
version of it...

int error;
struct resource tmp;
tmp .name = "ltlk";
tmps.start = 0x3F8;
tmp.end = 0x3FF;
tmp.flags = IORESOURCE_BUSY;
error = request_resource (&ioport_resource, &tmp);

The error returned is always -16. I looked at the code in kernel/resource.c 
where the request_region function is  defined. It builds a linked list of 
resources with start & end addresses. If you request a region that is 
already within the start-end range of a resource already in the list, it 
returns an error code. But it looks as if the region for a serial port, 
0x3f8 - 0x3ff,  in ioport_resource cannot be reserved because the entire 
range from 0x000 through 0xcf7 is already taken by something named "PCI Bus 
0000:00".  Therefore calling request_resource always fails and the driver 
for the speech synth errors out.

And therefore I can't use my hardware speech synth without modifying the 
kernel code.  If you comment out the line that checks the return code from 
request_region, it works. So you have to modify  the kernel code and compile 
a custom kernel to use a hardware speech synth. That's not such a problem 
for me but it is for a lot of people. Plus, the grml live CD doesn't work 
with hardware speech.  That is a problem for me.

Can anyone tell me how to fix this so it can be patched in the official 
kernel code?

Re: speakup bug

[Alan Cox]

On Sat, 3 Mar 2012 11:18:12 -0600
"John G. Heim" <jheim@math.wisc.edu> wrote:

> I need help fixing a bug in the driver for serial hardware  speech synths in 
> the speakup screen reader. According to the comments in the code, it is in a 
> part of the code that is trying to "steal" the serial port.

Yes - and the code is broken. To start with it's assumig a legacy PC
serial port at 0x3F8 and that it can beat the serial layer to it.

> returns an error code. But it looks as if the region for a serial port, 
> 0x3f8 - 0x3ff,  in ioport_resource cannot be reserved because the entire 
> range from 0x000 through 0xcf7 is already taken by something named "PCI Bus 
> 0000:00".  Therefore calling request_resource always fails and the driver 
> for the speech synth errors out.

It's a heirarchical space, so you can allocate things within it. Look
at /proc/ioports.

> And therefore I can't use my hardware speech synth without modifying the 
> kernel code.  If you comment out the line that checks the return code from 
> request_region, it works. So you have to modify  the kernel code and compile 
> a custom kernel to use a hardware speech synth. That's not such a problem 
> for me but it is for a lot of people. Plus, the grml live CD doesn't work 
> with hardware speech.  That is a problem for me.
> 
> Can anyone tell me how to fix this so it can be patched in the official 
> kernel code?

The proper fix is to make the drivers work via the serial layer properly.
The speakup people have been told this repeatedly for years and years
which is why their drivers work on less and less systems and won't run
with things PCI or USB serial ports, and why they are forever buried in
staging.

Alan

Re: speakup bug

[Ted Ts'o]

On Sat, Mar 03, 2012 at 05:39:16PM +0000, Alan Cox wrote:
> On Sat, 3 Mar 2012 11:18:12 -0600
> "John G. Heim" <jheim@math.wisc.edu> wrote:
> 
> > I need help fixing a bug in the driver for serial hardware  speech synths in 
> > the speakup screen reader. According to the comments in the code, it is in a 
> > part of the code that is trying to "steal" the serial port.
> 
> Yes - and the code is broken. To start with it's assumig a legacy PC
> serial port at 0x3F8 and that it can beat the serial layer to it.

Worse yet, it busy waits when sending characters to the UART.  Ugh...

> The proper fix is to make the drivers work via the serial layer properly.
> The speakup people have been told this repeatedly for years and years
> which is why their drivers work on less and less systems and won't run
> with things PCI or USB serial ports, and why they are forever buried in
> staging.

The driver desperately needs to be rewritten to attach to the serial
layer via a tty line discpline.  That way it will work on other types
of serial ports (i.e., PCI bus attached and USB devices).

   	  	       	       		    - Ted

Re: speakup bug

[John G. Heim]

From: Alan Cox
Sent: Saturday, March 03, 2012 11:39 AM
>> Can anyone tell me how to fix this so it can be patched in the official
>> kernel code?
>
>The proper fix is to make the drivers work via the serial layer properly.

Well, I am not the worlds greatest C programmer but many years ago I wrote 
unix
device drivers for a living. Could a reasonably competent person have a
chance of fixing that?

> The speakup people have been told this repeatedly for years and years
> which is why their drivers work on less and less systems and won't run
> with things PCI or USB serial ports, and why they are forever buried in
> staging.

Do you recall if they gave a reason for not wanting to use the serial layer?
I don't even know what the serial layer is (although the name is descriptive
enough and I can guess).But I would think they'd have had some reason for
not wanting to do that.

Re: speakup bug

[Samuel Thibault]

John G. Heim, le Sat 03 Mar 2012 18:01:07 -0600, a écrit :
> >The speakup people have been told this repeatedly for years and years

And the kernel hasn't really provided the feature for speakup to use it
for years and years.

> >which is why their drivers work on less and less systems and won't run
> >with things PCI or USB serial ports, and why they are forever buried in
> >staging.
> 
> Do you recall if they gave a reason for not wanting to use the serial layer?

Because no such facility really exists, it still has to to be done
before speakup can use it. It's on my TODO list for years now, and still
haven't found the time to work on it.

> I don't even know what the serial layer is (although the name is descriptive
> enough and I can guess).But I would think they'd have had some reason for
> not wanting to do that.

It's definitely not a "not want", but "can not use what does not exist".

Just for the record: what we need is being able to open a serial port
and read/write from inside the kernel. Using a daemon that does the open
and setting up speakup as a line discipline would be possible, but it
then depends on / working, and that's not something e.g. administrators
can really afford depending on.

Samuel

Re: speakup bug

[Samuel Thibault]

Samuel Thibault, le Sun 04 Mar 2012 01:18:06 +0100, a écrit :
> Just for the record: what we need is being able to open a serial port
> and read/write from inside the kernel. Using a daemon that does the open
> and setting up speakup as a line discipline would be possible, but it
> then depends on / working, and that's not something e.g. administrators
> can really afford depending on.

BTW: yes, that does not mean that speakup couldn't use a line discipline
already. Sure, still on the TODO list for years too.

Samuel

Re: speakup bug

[Ted Ts'o]

On Sun, Mar 04, 2012 at 01:21:11AM +0100, Samuel Thibault wrote:
> Samuel Thibault, le Sun 04 Mar 2012 01:18:06 +0100, a écrit :
> > Just for the record: what we need is being able to open a serial port
> > and read/write from inside the kernel. Using a daemon that does the open
> > and setting up speakup as a line discipline would be possible, but it
> > then depends on / working, and that's not something e.g. administrators
> > can really afford depending on.
> 
> BTW: yes, that does not mean that speakup couldn't use a line discipline
> already. Sure, still on the TODO list for years too.

Actualy you *can* open a tty from the kernel.  Look at how
/dev/console is opened in kernel_init() in init/main.c.  So you could
set up the line discipline out of the kernel if you really wanted to
get the speakup system set up in early boot, as opposed to setting
things up in either the initial ramdisk or in the init scripts.

If you don't even want a dependency on /, life gets a little harder,
but is that really that important?  The advantages of doing it cleanly
is that it will work on systems that don't have a normal serial port,
but only have a USB interface (for example, if you were going to try
to hack in support for a mobile device or some other embedded system).

   	   	       	 	       	  - Ted

Re: speakup bug

[Samuel Thibault]

Ted Ts'o, le Sat 03 Mar 2012 19:41:16 -0500, a écrit :
> On Sun, Mar 04, 2012 at 01:21:11AM +0100, Samuel Thibault wrote:
> > Samuel Thibault, le Sun 04 Mar 2012 01:18:06 +0100, a écrit :
> > > Just for the record: what we need is being able to open a serial port
> > > and read/write from inside the kernel. Using a daemon that does the open
> > > and setting up speakup as a line discipline would be possible, but it
> > > then depends on / working, and that's not something e.g. administrators
> > > can really afford depending on.
> > 
> > BTW: yes, that does not mean that speakup couldn't use a line discipline
> > already. Sure, still on the TODO list for years too.
> 
> Actualy you *can* open a tty from the kernel.  Look at how
> /dev/console is opened in kernel_init() in init/main.c.

But that still needs /dev

> If you don't even want a dependency on /, life gets a little harder,
> but is that really that important?

Yes. It does happen that / can not be mounted: disk encryption, disk
failure, nfs failure, etc. And without any feedback, it's very hard to
know what happened.

> The advantages of doing it cleanly
> is that it will work on systems that don't have a normal serial port,
> but only have a USB interface (for example, if you were going to try
> to hack in support for a mobile device or some other embedded system).

Sure!  But there should be a way to do it without /, just like the
serial console works on ttyUSB0 without /.

Samuel

Re: speakup bug

[Samuel Thibault]

Samuel Thibault, le Sun 04 Mar 2012 01:43:48 +0100, a écrit :
> Ted Ts'o, le Sat 03 Mar 2012 19:41:16 -0500, a écrit :
> > If you don't even want a dependency on /, life gets a little harder,
> > but is that really that important?
> 
> Yes. It does happen that / can not be mounted: disk encryption, disk
> failure, nfs failure, etc.

(Or simply bogus initrd)

> And without any feedback, it's very hard to know what happened.

Samuel

Re: speakup bug

[Ted Ts'o]

On Sun, Mar 04, 2012 at 01:43:48AM +0100, Samuel Thibault wrote:
> 
> Sure!  But there should be a way to do it without /, just like the
> serial console works on ttyUSB0 without /.

It's not impossible, but the hard part will be creating a naming
infrastructure so the user can specify the appropriate serial port if
there are multiple (non-console) tty devices present on the system.

My suggestion would be to get it working using a userspace daemon, and
then putting in the kernel code to open the serial port without a file
system.  It certainly can be done, with roughly the same techniques
that are used to mount the root file system in the non-initrd case.

     	      	       	   	     - Ted

Re: speakup bug

[Ted Ts'o]

On Sat, Mar 03, 2012 at 06:01:07PM -0600, John G. Heim wrote:
> 
> Do you recall if they gave a reason for not wanting to use the serial layer?
> I don't even know what the serial layer is (although the name is descriptive
> enough and I can guess).But I would think they'd have had some reason for
> not wanting to do that.

Looking at the code, I'm going to guess that it's largely historical.
I'd guess that original core code was originally written for MS-DOS
(main.c?  RLY?), and once they got it working, they never went back to
fix things.  Even if it wasn't the most efficient way to do things, I
suspect most of the blind community didn't care once it was working
--- and they probably didn't have anyone who really understood the tty
line discpline layer, since it wasn't very well documented.

	  	       	  	      - Ted

Re: speakup bug

[John G. Heim]

From: Alan Cox
Sent: Saturday, March 03, 2012 11:39 AM
Subject: Re: speakup bug

>> returns an error code. But it looks as if the region for a serial port,
>> 0x3f8 - 0x3ff,  in ioport_resource cannot be reserved because the entire
>> range from 0x000 through 0xcf7 is already taken by something named "PCI 
>> Bus
>> 0000:00".  Therefore calling request_resource always fails and the driver
> f>or the speech synth errors out.
>
>It's a heirarchical space, so you can allocate things within it. Look
>at /proc/ioports.

# file /proc/ioports
/proc/ioports: empty

>From looking at the code in resource.c, I don't know how you'd get a piece 
of an already allocated range.

It looks like fixing the speakup code to do serial ports right is a lot of 
work. Maybe we can just fix this problem in the mean time.

Re: speakup bug

[Alan Cox]

On Sat, 3 Mar 2012 20:03:48 -0600
"John G. Heim" <jheim@math.wisc.edu> wrote:

> From: Alan Cox
> Sent: Saturday, March 03, 2012 11:39 AM
> Subject: Re: speakup bug
> 
> >> returns an error code. But it looks as if the region for a serial port,
> >> 0x3f8 - 0x3ff,  in ioport_resource cannot be reserved because the entire
> >> range from 0x000 through 0xcf7 is already taken by something named "PCI 
> >> Bus
> >> 0000:00".  Therefore calling request_resource always fails and the driver
> > f>or the speech synth errors out.
> >
> >It's a heirarchical space, so you can allocate things within it. Look
> >at /proc/ioports.
> 
> # file /proc/ioports
> /proc/ioports: empty

Try cat on it not file