Re: fix for shared interrupt in serial i/o

[Russell King <rmk@arm.linux.org.uk>]

On Fri, Feb 01, 2002 at 12:12:33AM +0100, Stefani Seibold wrote:
> the serial driver in linux seems to have some troubles with
> shared serial interrupts in all know linux kernel.

Could you describe your serial port setup please?

> I don't know, who is currently the maintainer of the serial driver.
> Can anybody can tell me ;-)

I suppose that'd be me, but I'm not intending to be responsible for
the existing drivers, just the new driver that (hopefully) will make
it into 2.5 soonish.

Note that anything I've left out of the notes below doesn't mean that
that part is ok...

> If you like the patch, please apply it. I can make also this patch for
> kernel 2.4 and 2.5 available, if somebody want apply it to this
> kernel tree.

There are several things in this patch that could be simplified.
You can check the IRQ status of any port by reading one register -
the interrupt reason register (UART_IIR) and checking one bit.
This means you shouldn't need to modify receive_chars(),
transmit_chars(), nor check_modem_status(), and you'll be more
efficient.

> -#ifdef SERIAL_DEBUG_INTR
> +#ifndef SERIAL_DEBUG_INTR
>  	printk("rs_interrupt(%d)...", irq);
>  #endif

If serial interrupt debugging is not selected, you want to send messages
to the console?

> +	for(info=IRQ_ports[irq];info;info=info->next_port)
> +		serial_out(info, UART_IER, 0);
> +
> +	info = IRQ_ports[irq];
> +

Hmm, if we're scanning all ports on this IRQ line anyway, then why not
do this step within the main loop?

> -	info->timeout = ((info->xmit_fifo_size*HZ*bits*quot) / baud_base);
> -	info->timeout += HZ/50;		/* Add .02 seconds of slop */
> +	info->timeout = ((info->xmit_fifo_size*HZ*bits+(baud_base/quot-1))*quot) / baud_base;

I don't think you described your reasoning behind this change, however it
does look overly complex.  Unfortunately, its a potential division by zero
bug (when quot = 1).

Lets simplify it a bit anyway:

  (fifosize*HZ*bits*quot) / baudbase + (baudbase / quot - 1) * (quot / baudbase)
  (fifosize*HZ*bits*quot) / baudbase + (baudbase * quot) / (baudbase * (quot - 1))
  (fifosize*HZ*bits*quot) / baudbase + quot / (quot - 1)

And n/(n-1) will be:

  n  n/(n-1)
  1  infinity
  2  2
>=3  1

We used to always add HZ/50 (which comes out as '2') for all causes.

> +
> +	if (info->x_char || 

'x_char' isn't anything to do with the transmit queue - its the XON/XOFF
1 byte buffer.  I don't think you explained the purpose of these changes.

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html