Serial: updated serial drivers

Serial: updated serial drivers

[Russell King]

Hi,

I've been maintaining a serial driver "off the side" of the ARM port
which cleans up the serial driver mess that we currently have, with
many duplications of serial.c, each with subtle bugs.

Since ARM seems to have a poliferation of various UARTs, each of them
different, if this were to carry on we'd end up with 6 or 7 more
copies of serial.c.  This is obviously unacceptable.

So, with a view to cutting down on this duplication, I split serial.c
into a core driver, which can be used by various "low level" serial
hardware drivers, including the ARM ones, but also the 8250/16550
drivers.

Jeff and Arjan (iirc) expressed some concern about the PCI and PNP
code in serial.c, so that got separated out in what has now come to
be known as the serial rewrite.

The only non-ARM driver this patch affects is the 8250/16550 serial.c
driver.  The others have not been ported to this infrastructure yet.

The patch does contain details on the interface between the low level
and core serial drivers.

This project was first announced to linux-kernel on 6 November 2001:

 http://marc.theaimsgroup.com/?l=linux-kernel&m=100504194616062&w=2

Linus has expressed an interest in merging this work; hence this
message.  Before I do send it to Linus, I would like to get some
feedback on the code.  So, here is the latest patch against 2.5.24:

 http://www.home.arm.linux.org.uk/cvs/serial-2.5.24.diff.bz2

It should cleanly apply to 2.5.24.  The names of the serial options
have changed, so use your favourite configuration program to select
the relevant options.

This patch affects the following mainline drivers:
	- 8250/16550 "generic" serial driver (serial.o)
	- PCMCIA serial probe module (serial_cs.o)

Please send any bug reports in my direction.  There is one specific
area that I'm unable to test here - 8250/16x50 "shared" interrupts
(where multiple 8250/16x50 devices share the same interrupt line).
I'm expecting my mailbox to overflow tomorrow, so...

Note that, as ever, 8250/16x50 sharing with non-serial devices is
NOT guaranteed to work on edge-based IRQ systems like PCs (and if
you want this to work you can either configure your kernel
appropriately, or pass "share_irqs=1" in modules.conf for
serial_8250.o)

The future:

 - work with tytso to integrate any relevant bits of serial_core.c
   with the tty layer.
 - fix bugs
 - there seems to be a fair number of people wanting support for
   the higher speeds of UARTs (eg 16C950 and most motherboard
   devices)
 - add support for driverfs
 - remove the silly situation where we have ports openable, but
   no hardware behind them (and use some other method to create
   serial devices on demand if still required)
 - remove callout functionality, which has been marked as such for
   a few years now.  tytso mentions that the only program this
   might break is minicom, which should be fixed.

I'm sure there's other stuff people want. 8)

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

Re: Serial: updated serial drivers

[Miquel van Smoorenburg]

In article <20020707010009.C5242@flint.arm.linux.org.uk>,
Russell King  <rmk@arm.linux.org.uk> wrote:
> - remove callout functionality, which has been marked as such for
>   a few years now.  tytso mentions that the only program this
>   might break is minicom, which should be fixed.

Minicom doesn't care which port you use. It might be that some
people have saved configs of 6 years old that still contain
references to a cua device, but that's a problem any serial
program has.

>From the manpage of any recent minicom:

       Serial port setup
          *A - Serial device
               /dev/tty1   or   /dev/ttyS1   for   most   people.
               /dev/cua<n> is still possible under linux, but not
               recommended  any  more  because  these devices are
               obsolete and many  newly  installed  systems  with
               kernel  2.2.x  don't  have them.  Use /dev/ttyS<n>
               instead.

Mike.

Re: Serial: updated serial drivers

[William Lee Irwin III]

On Sun, Jul 07, 2002 at 01:00:09AM +0100, Russell King wrote:
> I've been maintaining a serial driver "off the side" of the ARM port
> which cleans up the serial driver mess that we currently have, with
> many duplications of serial.c, each with subtle bugs.

global_cli() overhead on my testbox is a significant problem.

Profile info from tbench 1024 with ttyS0 as stdout, taken on a 16 cpu
i386 box with 16GB of RAM and irqbalance disabled, (needed to boot):

90372662 total                                    713.6412
44801051 default_idle                             861558.6731
36220921 mod_timer                                113190.3781
2510075 timer_bh                                 2764.3998
2344795 __global_cli                             8620.5699
1446315 __wake_up                                7693.1649
1370742 do_gettimeofday                          10078.9853
924996 schedule                                 831.8309
512368 do_softirq                               2328.9455
103136 tasklet_hi_action                        526.2041
 40640 system_call                              923.6364
 19238 do_page_fault                             14.2927
 12835 add_wait_queue                           103.5081
 10667 remove_wait_queue                         83.3359
  7990 bh_action                                 38.4135
  5303 pte_alloc_one                             27.6198
  4665 schedule_timeout                          29.1562
  4584 pgd_alloc                                 24.3830
  3870 syscall_call                             351.8182
  3633 try_to_wake_up                             6.3073
  3100 exit_notify                                3.5068
  2566 del_timer                                 14.9186

The disabling of irqbalance should make these profiling results valid.


Cheers,
Bill

Re: Serial: updated serial drivers

[William Lee Irwin III]

On Sun, Jul 07, 2002 at 01:00:09AM +0100, Russell King wrote:
>> I've been maintaining a serial driver "off the side" of the ARM port
>> which cleans up the serial driver mess that we currently have, with
>> many duplications of serial.c, each with subtle bugs.

On Mon, Jul 15, 2002 at 03:03:10AM -0700, William Lee Irwin III wrote:
> global_cli() overhead on my testbox is a significant problem.
> Profile info from tbench 1024 with ttyS0 as stdout, taken on a 16 cpu
> i386 box with 16GB of RAM and irqbalance disabled, (needed to boot):

the profiling results were for a kernel without the new serial stuff.
The new serial stuff appears to need some arch compatibility auditing/
fixes for NUMA-Q.



Cheers,
Bill

Re: Serial: updated serial drivers

[Russell King]

On Mon, Jul 15, 2002 at 04:01:35AM -0700, William Lee Irwin III wrote:
> the profiling results were for a kernel without the new serial stuff.
> The new serial stuff appears to need some arch compatibility auditing/
> fixes for NUMA-Q.

I am not aware of any architecture specific code in the new serial
code, with the exception of a couple of writes to port 0x80 for i386
architectures (which the original serial.c driver did as well.)

Can you give an idea where it fails/kernel messages please?

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

Re: Serial: updated serial drivers

[Dave Hansen]

William Lee Irwin III wrote:
> On Sun, Jul 07, 2002 at 01:00:09AM +0100, Russell King wrote:
> 
>>I've been maintaining a serial driver "off the side" of the ARM port
>>which cleans up the serial driver mess that we currently have, with
>>many duplications of serial.c, each with subtle bugs.
> 
> 
> global_cli() overhead on my testbox is a significant problem.
> 
> Profile info from tbench 1024 with ttyS0 as stdout, taken on a 16 cpu
> i386 box with 16GB of RAM and irqbalance disabled, (needed to boot):
> 
<snip profile>
 >
> The disabling of irqbalance should make these profiling results valid.

So, is irqbalance the thing that is screwing up our profiles on 2.5? 
We were getting some strage profiles that made us look at oprofile 
again.

oprofile is really cool, but readprofile is dead simple.
-- 
Dave Hansen
haveblue@us.ibm.com

Re: Serial: updated serial drivers

[William Lee Irwin III]

On Mon, Jul 15, 2002 at 04:01:35AM -0700, William Lee Irwin III wrote:
>> the profiling results were for a kernel without the new serial stuff.
>> The new serial stuff appears to need some arch compatibility auditing/
>> fixes for NUMA-Q.

On Mon, Jul 15, 2002 at 12:24:55PM +0100, Russell King wrote:
> I am not aware of any architecture specific code in the new serial
> code, with the exception of a couple of writes to port 0x80 for i386
> architectures (which the original serial.c driver did as well.)
> Can you give an idea where it fails/kernel messages please?

It's not obvious to me, either, I actually read the stuff and didn't
see where the problem could be.

It appeared to fail before console_init(). The system is largely
dedicated to other projects so there will be a bit of turnaround time
between test runs. I'll probably be able to pinpoint the point of
failure later tonight.


Cheers,
Bill

Re: Serial: updated serial drivers

[Marko Kohtala]

Russell King wrote:
> Linus has expressed an interest in merging this work; hence this
> message.  Before I do send it to Linus, I would like to get some
> feedback on the code.  So, here is the latest patch against 2.5.24:
> 
>  http://www.home.arm.linux.org.uk/cvs/serial-2.5.24.diff.bz2
...
> I'm sure there's other stuff people want. 8)

It looks good.

I have one additional request that might best be done while doing this kind of rewrite.

I came across an IndustryPack (IP) carrier card. Each IP sits on a 16 bit bus. The carrier is a PCI device and has slots for four IP, each IP module I had carried eight 16550 serial ports.

With this I came up with two problems:

1) The PCI<->IP bridge had a bug with byte accesses. The iotype SERIAL_IO_MEM and iomem_reg_shift 1 addressed right addresses, but with byte access, so it did not work.

2) All the 32 serial ports were on same IRQ. You could find out which ports need service on an interrupt with one or two register reads, but the serial driver wanted to get and handle the IRQ on its own. The multiport support did not quite work for this.

One solution to allow the carrier driver address these would be to replace the struct uart_port iotype with a pointer to

struct serial_hw_ops {
  unsigned int (*serial_in)(struct uart_port *, int offset);
  void (*serial_out)(struct uart_port *, int offset, int value);
  int (*enable_irq)(struct uart_port *, void (*irq_func)(struct uart_port *));
  void (*disable_irq)(struct uart_port *);
};

Then implement new register_serial functions for the carrier card driver to use that take this as argument (serial8250_register?). The irq_func would be the function carrier driver calls to get the IRQ serviced (close to serial8250_handle_port).

The struct uart_port would need to also carry a pointer to carrier specific data. Only the enable_irq and disable_irq will need carrier data.

I think the current iobase and other members are good enough such that the serial_in and serial_out can be implemented rather efficiently without the extra indirection through the carrier data pointer.

I'm not sure if it would still incur a small overhead to I/O access. Perhaps not, as it would seem to me the serial_in does not get inlined and there would be one switch statement we could avoid by this. You are more competent to judge this.

This would also nicely clean up the SERIAL_IO_HUB6 case in serial_in/out. And it would help other multiport serial cards to utilize the registers they have to isolate the ports that interrupt.

The enable_irq in the carrier driver would see if the port is the first on the carrier, and request the IRQ to its own interrupt function. On the same interrupt there can be other things besides the serial ports.

struct serial_hw_ops instances and the functions for the standard single port or standard multiport cases should be exported. The carrier card has it's own module that needs access to these things for the serial8250_register call.

There are IP with other UART than 16550, so this should be used for all UART drivers.

Since this seems to affect the interface to port registration functions, it might be better to have it done now rather than later.


............................................................
Maksuton sähköposti aina käytössä http://luukku.com                            
Kuukausimaksuton MTV3 Internet-liittymä www.mtv3.fi/liittyma