[PATCH] Add the ability to layer another driver over the serial driver

[PATCH] Add the ability to layer another driver over the serial driver

[Corey Minyard]

This is a set of three patches to allow adding another driver on top of
the current serial driver without too much change to the serial code.
This is more for comments right now, it is probably not ready for real
use yet.

The patches are too big to post here, so I'm putting them on
http://home.comcast.net/~minyard

The three patches are:

    * serial-remove-tty-struct-from-driver.patch - A general patch to
      remove the tty includes from the low-level serial drivers. Only
      fixes the 8250 for now.

    * serial-allow-in-kernel-users.patch - The actual patch that adds
      the layered driver to the serial core.

    * serial-8250-cleanup.patch - Add support for the layered driver
      and poll to the 8250 uart.

This is for a serial interface to an IPMI controller.  Note that I
don't really like this very much for a number of reasons, but the IPMI
driver has features that only work in-kernel, like extending the
watchdog timer on a panic to allow kdump to operate without a reboot
and storing panic information in the IPMI event log.  So you really
can't write a userland-only interface for this and keep those
features.

It is also not really possible to add this on top of the tty code
because it has to be able to run at panic time, and the tty code
doesn't have a clean place to interface this from what I could
tell.

-Corey

Re: [PATCH] Add the ability to layer another driver over the serial driver

[Tilman Schmidt]

On Mon, 16 Oct 2006 11:53:15 -0500, Corey Minyard <cminyard@mvista.com> wrote:
> This is a set of three patches to allow adding another driver on top of
> the current serial driver without too much change to the serial code.
> This is more for comments right now, it is probably not ready for real
> use yet.
> 
> The patches are too big to post here, so I'm putting them on
> http://home.comcast.net/~minyard
> 
> The three patches are:
> 
>     * serial-remove-tty-struct-from-driver.patch - A general patch to
>       remove the tty includes from the low-level serial drivers. Only
>       fixes the 8250 for now.
> 
>     * serial-allow-in-kernel-users.patch - The actual patch that adds
>       the layered driver to the serial core.
> 
>     * serial-8250-cleanup.patch - Add support for the layered driver
>       and poll to the 8250 uart.

Has anything ever come of this? I would be very much interested in it.
It might make it possible to extend the Siemens Gigaset drivers
(drivers/isdn/gigaset) to the RS232 attached M101 DECT adapter.
There is a working driver out of tree which accesses the serial port
hardware directly (i8250 only), but that kind of thing doesn't seem
fit for inclusion in the kernel.

Thanks
Tilman

Re: [PATCH] Add the ability to layer another driver over the serial driver

[Guennadi Liakhovetski]

On Sun, 10 Dec 2006, Tilman Schmidt wrote:

> On Mon, 16 Oct 2006 11:53:15 -0500, Corey Minyard <cminyard@mvista.com> wrote:
> > This is a set of three patches to allow adding another driver on top of
> > the current serial driver without too much change to the serial code.
> > This is more for comments right now, it is probably not ready for real
> > use yet.

[snip]

> Has anything ever come of this? I would be very much interested in it.
> It might make it possible to extend the Siemens Gigaset drivers
> (drivers/isdn/gigaset) to the RS232 attached M101 DECT adapter.
> There is a working driver out of tree which accesses the serial port
> hardware directly (i8250 only), but that kind of thing doesn't seem
> fit for inclusion in the kernel.

...FWIW I would also gladly remove 
arch/powerpc/platforms/embedded6xx/ls_uart.c 
in favour of a standard interface to drivers/serial.

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: [PATCH] Add the ability to layer another driver over the serial driver

[Corey Minyard]

Nothing has come of this yet.  But we have these two requests and a 
request from Russell Doty at Redhat.

It would be nice to know if this type of thing was acceptable or not, 
and the problems with the patch.  The patch is at 
http://home.comcast.net/~minyard

-Corey

Guennadi Liakhovetski wrote:
> On Sun, 10 Dec 2006, Tilman Schmidt wrote:
>
>   
>> On Mon, 16 Oct 2006 11:53:15 -0500, Corey Minyard <cminyard@mvista.com> wrote:
>>     
>>> This is a set of three patches to allow adding another driver on top of
>>> the current serial driver without too much change to the serial code.
>>> This is more for comments right now, it is probably not ready for real
>>> use yet.
>>>       
>
> [snip]
>
>   
>> Has anything ever come of this? I would be very much interested in it.
>> It might make it possible to extend the Siemens Gigaset drivers
>> (drivers/isdn/gigaset) to the RS232 attached M101 DECT adapter.
>> There is a working driver out of tree which accesses the serial port
>> hardware directly (i8250 only), but that kind of thing doesn't seem
>> fit for inclusion in the kernel.
>>     
>
> ...FWIW I would also gladly remove 
> arch/powerpc/platforms/embedded6xx/ls_uart.c 
> in favour of a standard interface to drivers/serial.
>
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski
>   

Re: [PATCH] Add the ability to layer another driver over the serial driver

[Alan]

On Sun, 10 Dec 2006 19:23:54 -0600
Corey Minyard <cminyard@mvista.com> wrote:

> Nothing has come of this yet.  But we have these two requests and a 
> request from Russell Doty at Redhat.
> 
> It would be nice to know if this type of thing was acceptable or not, 
> and the problems with the patch.  The patch is at 
> http://home.comcast.net/~minyard

This looks wrong. You already have a kernel interface to serial drivers.
It is called a line discipline. We use it for ppp, we use it for slip, we
use it for a few other things such as attaching sync drivers to some
devices.

Discussions of the form  "my line discipline has no way to do 'xyz'" are
the ones that need to happen IMHO.

Alan

Re: [PATCH] Add the ability to layer another driver over the serial driver

[Corey Minyard]

Alan wrote:
> On Sun, 10 Dec 2006 19:23:54 -0600
> Corey Minyard <cminyard@mvista.com> wrote:
>
>   
>> Nothing has come of this yet.  But we have these two requests and a 
>> request from Russell Doty at Redhat.
>>
>> It would be nice to know if this type of thing was acceptable or not, 
>> and the problems with the patch.  The patch is at 
>> http://home.comcast.net/~minyard
>>     
>
> This looks wrong. You already have a kernel interface to serial drivers.
> It is called a line discipline. We use it for ppp, we use it for slip, we
> use it for a few other things such as attaching sync drivers to some
> devices.
>
> Discussions of the form  "my line discipline has no way to do 'xyz'" are
> the ones that need to happen IMHO.
>   
Thanks.  Line disciplines did not seem suitable for what I
needed, but perhaps can be adapted.

So here's the start of discussion:

1) The IPMI driver needs to run at panic time to modify watchdog
timers and store panic information in the event log.  So no work
queues, no delayed work, and the need for some type of poll
operation on the device.

2) The IPMI driver needs to get messages through even when
the system is very busy.  Since a watchdog runs over the driver,
it needs to be able to get messages through to avoid a system
reset as long as something is pinging the watchdog from userland.

Currently there are mutexes, lock_kernel() calls, and work queues
that cause trouble for these.

There is also a comment that you can't set low_latency and call
tty_flip_buffer_push from IRQ context.  This seems to be due to a
lack of anything in flush_to_ldisc to handle reentrancy, and perhaps
because disc->receive_buf can block, but I couldn't tell easily.

-Corey

Re: [PATCH] Add the ability to layer another driver over the serial driver

[Alan]

On Mon, 11 Dec 2006 08:52:20 -0600
Corey Minyard <cminyard@mvista.com> wrote:
> So here's the start of discussion:
> 
> 1) The IPMI driver needs to run at panic time to modify watchdog
> timers and store panic information in the event log.  So no work
> queues, no delayed work, and the need for some type of poll
> operation on the device.

That would be the existing serial console interface. For things like USB
serial you are probably out of luck. At the moment we have a single
"console" attached to a specific serial console interface. The code is
almost all there for allowing other parties to create new synchronous
serial logging devices by opening open as the console driver does.

> 2) The IPMI driver needs to get messages through even when
> the system is very busy.  Since a watchdog runs over the driver,
> it needs to be able to get messages through to avoid a system
> reset as long as something is pinging the watchdog from userland.

You have a high priority character output function which jumps existing
data. Not all hardware implements this, and some of the modern hardware
in particular simply doesn't physically support such behaviour. Also if
you are the sole user of the port you *know* nobody else will be queuing
data to it anyway.

> Currently there are mutexes, lock_kernel() calls, and work queues
> that cause trouble for these.
> 
> There is also a comment that you can't set low_latency and call
> tty_flip_buffer_push from IRQ context.  This seems to be due to a
> lack of anything in flush_to_ldisc to handle reentrancy, and perhaps
> because disc->receive_buf can block, but I couldn't tell easily.

The entire tty side locking, scheduling and design is based upon the idea
that input processing is deferred. Otherwise you get long chains of
recursion from the worst cases.

Alan

Re: [PATCH] Add the ability to layer another driver over the serial driver

[Corey Minyard]

Alan wrote:
> On Mon, 11 Dec 2006 08:52:20 -0600
> Corey Minyard <cminyard@mvista.com> wrote:
>   
>> So here's the start of discussion:
>>
>> 1) The IPMI driver needs to run at panic time to modify watchdog
>> timers and store panic information in the event log.  So no work
>> queues, no delayed work, and the need for some type of poll
>> operation on the device.
>>     
>
> That would be the existing serial console interface. For things like USB
> serial you are probably out of luck. At the moment we have a single
> "console" attached to a specific serial console interface. The code is
> almost all there for allowing other parties to create new synchronous
> serial logging devices by opening open as the console driver does.
>   
I don't think the console interface really helps.  Input processing is also
needed, and the output blocks until the character is sent.  A poll routine
is a better way to accomplish this, IMHO.
>   
>> 2) The IPMI driver needs to get messages through even when
>> the system is very busy.  Since a watchdog runs over the driver,
>> it needs to be able to get messages through to avoid a system
>> reset as long as something is pinging the watchdog from userland.
>>     
>
> You have a high priority character output function which jumps existing
> data. Not all hardware implements this, and some of the modern hardware
> in particular simply doesn't physically support such behaviour. Also if
> you are the sole user of the port you *know* nobody else will be queuing
> data to it anyway.
>   
That's not really the issue.  The input processing is again the
issue.
>   
>> Currently there are mutexes, lock_kernel() calls, and work queues
>> that cause trouble for these.
>>
>> There is also a comment that you can't set low_latency and call
>> tty_flip_buffer_push from IRQ context.  This seems to be due to a
>> lack of anything in flush_to_ldisc to handle reentrancy, and perhaps
>> because disc->receive_buf can block, but I couldn't tell easily.
>>     
>
> The entire tty side locking, scheduling and design is based upon the idea
> that input processing is deferred. Otherwise you get long chains of
> recursion from the worst cases.
>   
I was actually wrong, flush_to_ldisc does handle reentrancy.
It can only have one caller to disc->receive_buf() at a time.  So
long chains of recursion don't seem to be possible, even if called
from IRQ context.

And studying the way ppp does writing, it can bypass the tty_write()
call and directly call the drivers.  So that bypasses the transmit
locking problems I saw.

This is going to require some more thought.  But I believe it can be
done with adding a poll routine to the tty_operations structure and
perhaps some minor changes around tty_flip_buffer_push.

Thanks for your help.

-Corey

Re: [PATCH] Add the ability to layer another driver over the serial driver

[Alan]

> I was actually wrong, flush_to_ldisc does handle reentrancy.
> It can only have one caller to disc->receive_buf() at a time.  So
> long chains of recursion don't seem to be possible, even if called
> from IRQ context.

disc->receive_buf is single threaded but if it then sends characters back
in the same context (eg flow control) you get re-entry in the driver.

> And studying the way ppp does writing, it can bypass the tty_write()
> call and directly call the drivers.  So that bypasses the transmit
> locking problems I saw.

tty_write() is the layer above the ldisc. tty_write() feeds a line
discipline from (usually) user space. Line disciplines write direct to
the tty.

> 
> This is going to require some more thought.  But I believe it can be
> done with adding a poll routine to the tty_operations structure 

What status do you need to poll ?

Re: [PATCH] Add the ability to layer another driver over the serial driver

[Corey Minyard]

Alan wrote:
>> This is going to require some more thought.  But I believe it can be
>> done with adding a poll routine to the tty_operations structure 
>>     
>
> What status do you need to poll ?
>   
I need to poll for receive and transmit characters so I can do I/O
during panics (interrupts disabled).

Re: [PATCH] Add the ability to layer another driver over the serial driver

[Corey Minyard]

This is a continuation of a previous discussion.  There are
serial interfaces to IPMI chips that I need to support and
I need a way to access these at panic time or when the
system is in a state where it can't schedule.

I have written a layered driver for the serial core, but Alan
says that a line discipline is the right way to have a driver
access a serial port.  That seems rather unnatural to me
(none of the other layered drivers do this), but it's not the
end of the world, and the serial driver is "special" in some
regards.

Alan and I discussed this some and he suggested I look at a
way to unify all the various "raw" users of serial ports, things
like kgdb, serial console, and the IPMI driver.  That sounded
like a good idea at the time, so I've been working on that.

Unfortunately, it hasn't turned out so well.  A line discipline
won't work for kgdb or consoles; the driver needs some
mechanism to register the serial port so kgdb can find it.
Consoles need some special registration, too.

This is what I would like to propose:  Keep the concept of
a layered driver like I have already done, but modify the
code so the uart can register very early, in time for kgdb
and consoles.  Add a uart poll routine, poll setup and
poll quit routine, and a few other things needed by
consoles.  kgdb can use this, and the serial_core code
can use it to do a console.

This would eliminate most of the console code from the
individual drivers.  The normal write buffering could be
used by the serial console (though it would have to
insert its own info structure when it did a console write).
The console also needs routines to get the default values
for systems that get defaults through things like open
firmware.  And maybe a routine for polling CTS.

The poll routines could be propagated up to the tty
layer fairly easily, if required, for use by other drivers.
I really don't like having to have two mechanisms for
a driver to do a write, one normal and one when the
system can't schedule.  And a poll routine reduces
code duplication, the driver works with all the normal
routines.  No special read or write operations.

Anyway, this is what I think works best for all parties.
I'll continue down this path for now.

Thanks,

-Corey

Re: [PATCH] Add the ability to layer another driver over the serial driver

[Tilman Schmidt]

[-- Attachment #1: Type: text/plain, Size: 786 bytes --]

On Mon, 11 Dec 2006 10:20:16 +0000, Alan wrote:
> This looks wrong. You already have a kernel interface to serial drivers.
> It is called a line discipline. We use it for ppp, we use it for slip, we
> use it for a few other things such as attaching sync drivers to some
> devices.

I was under the impression that line disciplines need a user space
process to open the serial device and push them onto it. Is there
a way for a driver to attach to a serial port through the line
discipline interface from kernel space, eg. from an initialization,
module load, or probe function?

-- 
Tilman Schmidt                    E-Mail: tilman@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

Re: [PATCH] Add the ability to layer another driver over the serial driver

[Corey Minyard]

Tilman Schmidt wrote:
> On Mon, 11 Dec 2006 10:20:16 +0000, Alan wrote:
>   
>> This looks wrong. You already have a kernel interface to serial drivers.
>> It is called a line discipline. We use it for ppp, we use it for slip, we
>> use it for a few other things such as attaching sync drivers to some
>> devices.
>>     
>
> I was under the impression that line disciplines need a user space
> process to open the serial device and push them onto it. Is there
> a way for a driver to attach to a serial port through the line
> discipline interface from kernel space, eg. from an initialization,
> module load, or probe function?
>   
Module initialization functions run in a task context, so that's
generally not a problem.  The probe function depends on the driver,
I guess, but most I have seen are in task context.

-Corey

Re: [PATCH] Add the ability to layer another driver over the serial driver

[Tilman Schmidt]

[-- Attachment #1: Type: text/plain, Size: 1336 bytes --]

Am 11.12.2006 18:07 schrieb Corey Minyard:
> Tilman Schmidt wrote:
>> I was under the impression that line disciplines need a user space
>> process to open the serial device and push them onto it. Is there
>> a way for a driver to attach to a serial port through the line
>> discipline interface from kernel space, eg. from an initialization,
>> module load, or probe function?
>>   
> Module initialization functions run in a task context, so that's
> generally not a problem.  The probe function depends on the driver,
> I guess, but most I have seen are in task context.

Could you be a bit more specific? If I write a module implementing a
line discipline, how would I go about having that line discipline
push itself onto a given serial port (specified for example through a
module parameter) immediately, during its own module initialization?
I can't seem to find an in-kernel interface for that.

Also, if I understand correctly, this would only work if the driver
is compiled as a module, but such a limitation seems to be frowned
upon within the kernel community. Any way around that?

Thanks
Tilman

-- 
Tilman Schmidt                          E-Mail: tilman@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 253 bytes --]

Re: [PATCH] Add the ability to layer another driver over the serial driver

[Alan]

On Mon, 11 Dec 2006 17:58:29 +0100
Tilman Schmidt <tilman@imap.cc> wrote:

> On Mon, 11 Dec 2006 10:20:16 +0000, Alan wrote:
> > This looks wrong. You already have a kernel interface to serial drivers.
> > It is called a line discipline. We use it for ppp, we use it for slip, we
> > use it for a few other things such as attaching sync drivers to some
> > devices.
> 
> I was under the impression that line disciplines need a user space
> process to open the serial device and push them onto it. 

Yes that is correct. You need a way for the user to tell you which port
to use and to the permission and usage management for it anyway (as well
as load the module and configure settings), so this seems quite
reasonable.

Re: [PATCH] Add the ability to layer another driver over the serial driver

[Tilman Schmidt]

[-- Attachment #1: Type: text/plain, Size: 1886 bytes --]

Am 11.12.2006 18:40 schrieb Alan:
> On Mon, 11 Dec 2006 17:58:29 +0100
> Tilman Schmidt <tilman@imap.cc> wrote:
> 
>> On Mon, 11 Dec 2006 10:20:16 +0000, Alan wrote:
>>> This looks wrong. You already have a kernel interface to serial drivers.
>>> It is called a line discipline. We use it for ppp, we use it for slip, we
>>> use it for a few other things such as attaching sync drivers to some
>>> devices.
>> I was under the impression that line disciplines need a user space
>> process to open the serial device and push them onto it. 
> 
> Yes that is correct. You need a way for the user to tell you which port
> to use and to the permission and usage management for it anyway (as well
> as load the module and configure settings), so this seems quite
> reasonable.

I'm afraid I'm not familiar with Linux' SLIP implementation.
So there's a line discipline called "slip" which, when pushed
onto a serial port, registers as a network device, right? How
does it get - and stay - pushed? Is there a daemon process which
opens the serial port, pushes the line discipline onto it, and
then just sleeps, keeping the serial port open so that the line
discipline stays put? Can you point me to the source for such a
daemon for reference?

What I am actually looking for is a way to port an existing driver
which directly programs an i8250 serial port, to cooperate more
cleanly with the existing serial port drivers of Linux (and, at
the same time, shed the dependency on the specific serial port
hardware.) If that requires a permanently running (or sleeping)
userspace daemon, then so be it. (Although I admit I'd rather do
without.)

Thanks
Tilman

-- 
Tilman Schmidt                          E-Mail: tilman@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 253 bytes --]

Re: [PATCH] Add the ability to layer another driver over the serial driver

[Guennadi Liakhovetski]

On Mon, 11 Dec 2006, Alan wrote:

> On Sun, 10 Dec 2006 19:23:54 -0600
> Corey Minyard <cminyard@mvista.com> wrote:
> 
> > Nothing has come of this yet.  But we have these two requests and a 
> > request from Russell Doty at Redhat.
> > 
> > It would be nice to know if this type of thing was acceptable or not, 
> > and the problems with the patch.  The patch is at 
> > http://home.comcast.net/~minyard
> 
> This looks wrong. You already have a kernel interface to serial drivers.
> It is called a line discipline. We use it for ppp, we use it for slip, we
> use it for a few other things such as attaching sync drivers to some
> devices.

Alan, my understanding might be wrong, but, I think, line disciplines are 
there as "protocols" for user-tty interfaces, i.e., you need a user, that 
opens a tty, sets a line discipline to it, and does io (read/write) over 
it, and NOT to be completely initialised and driven from the kernel. 
Whereas, what some users need is a complete in-kernel interface, when 
either another driver (like in case with the previous poster), or the 
platform code (linkstation) know that there's a device attached to this 
UART, know how and WHEN to operate it, and the user doesn't care about it 
at all. Think of it as about, say, i2c devices, that have user device 
interface and in-kernel interface, to which you can connect rtc, USB 
transceivers, that get controlled completely from the kernel TRANSPARENTLY 
for the user.

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: [PATCH] Add the ability to layer another driver over the serial driver

[Alan]

> there as "protocols" for user-tty interfaces, i.e., you need a user, that 
> opens a tty, sets a line discipline to it, and does io (read/write) over 
> it, and NOT to be completely initialised and driven from the kernel.

Take a look at the SLIP driver. User space sets up the port but all the
actual I/O is to/from the kernel not via user space.

Re: [PATCH] Add the ability to layer another driver over the serial driver

[Guennadi Liakhovetski]

On Mon, 11 Dec 2006, Alan wrote:

> > there as "protocols" for user-tty interfaces, i.e., you need a user, that 
> > opens a tty, sets a line discipline to it, and does io (read/write) over 
> > it, and NOT to be completely initialised and driven from the kernel.
> 
> Take a look at the SLIP driver. User space sets up the port but all the
> actual I/O is to/from the kernel not via user space.

Ok, to be specific, linkstation-like mpc8241-based NAS systems have an AVR 
perform power management functions, and it is connected over a UART. Some 
of the functions you have to perform:

1) disable the watchdog at startup otherwise it powers the system down in 
a few minutes. This is done by sending about 20 bytes to it - more than 
fits in UART's fifo. So, you need interrupts / polling. It is arguable 
whether this should be done / initialised from the user- or from the 
kernel-space. Earlier it was done from the user space completely, I put it 
into the kernel completely to get a runnable system even without 
user-space hacks.

2) run-time operation, like fan control, sensors, maybe some other 
functions. These all should and are currently done completely from a 
user-space process over a normal tty.

3) reboot / halt. Consists of 2 parts. The actual command, that can be 
sent earlier from the userspace too, and the final "commit", that actually 
reboots / powers down the system. That is the last command in platform's 
reboot / poweroff functions. Currently I do both parts from the kernel so 
with the current kernel you get a completely functional system with "any" 
generic distribution.

So, if we decide to put 1) in the user-space too, it becomes almost a 
normal line discipline with the only need to initiate io from the kernel 
for 3), which is just sending about 4 bytes over the port anyway, and can 
be (and is) just done in a tight infinite loop. But I'd prefer to keep 1) 
and 3) in the kernel and perform it without waiting for any user-space 
daemons...

What do you think?

Thanks
Guennadi
---
Guennadi Liakhovetski