Re: Bogus serialP.h patch?

Theodore Y. Ts'o (tytso@MIT.EDU)
Wed, 3 Nov 1999 13:03:36 -0500


Date: Wed, 3 Nov 1999 08:43:11 -0800 (PST)
From: Linus Torvalds <torvalds@transmeta.com>

Actually, I've always hated the notion of "public" versus "private", and I
think "serialP.h" is just a band-aid around a real problem which is that
there is quite a lot of incestuous knowledge about the tty layer and
serial devices all over the place.

Actually, there shouldn't be anything in serialP.h which is used by the
tty layer. The layering is actually pretty good, although it is
complex, given that tty's as defined by POSIX.1 are complex beasts.

I created serialP.h simply to clean up serial.c by moving the header
files outside of serialP.h. I don't know who added serialP.h to tty.h,
way back when, but it wasn't me. And as far as I know, none of the tty
layer should know or care about any of the definitions of serialP.h.

First off, if it's really a _private_ header file, then it shouldn't be in
<linux/serialP.h> in the first place. It should be in drivers/char, and
you should use #include "16550.h" to make it clear that (a) it's not about
"serial devices", it's about a specific _class_ of serial devices and (b)
it's really just private to a specific driver (or two similar drivers),
and not a generic Linux header file.

Agreed. serialP.h was created before it was common place to put header
files outside of the include hierarchy, and is basically an historical
accident. Given 20/20 hindsight, perhaps I shouldn't have created the
d*mn header file at all, and just put it all into serial.c, since
no other file should be depending on it anyway.

Btw, calling the dang thing "16550.h" may be technically inaccurate (it
obviously is used a lot more chips than the 16550), but I think it is
_psychologically_ a lot closer to what you seem to have in mind for the
use. It would tell people what the file is about - which the current name
does not at all. The current name probably makes most people think that
the programmer was spastic and wrote an extra 'P' by mistake.

Well, the whole name "serial.c" is part of the problem; perhaps we would
have been better off calling it "ns8250.c". But as you know, that's
also part of the historical baggage. The "P" convention to distinguish
private header files from public interface files is not an unusual
convention, and IMHO, is one that we should use more often. Information
hiding is a good thing.

But I don't care all that much. I think that either we should do the
one-liner to make existing things happy as things stand (which is
basically what Alan did), or we should just fix the thing _right_, in
which case the "serialP.h" file goes away - moved or integrated, I don't
much think there is all that much of a difference (the integration should
be much more complete, with a "serial device layer" kind of support
structure etc).

One of the things which has been on my todo list for a long time, but
which has languished due to lack of time, has been to move more
functionality into the tty layer. Break handling and tty baud
calculations have already been moved into the tty layer; the same goes
for the tty flip buffers, and so on. That's probably far cleaner than
creating a new "serial device layer". But other than that, I agree.

(Although it *wasn't* just a one-line change; this was a wholesale
movement of a hunk of declarations from serialP.h to serial.h

- Ted

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/