As reported by Gustavo de Nardin <[email protected]>, while trying to
compile xosview (http://xosview.sourceforge.net/) with upstream kernel
headers being used you get the following errors:
serialmeter.cc:48:30: error: linux/serial_reg.h: No such file or directory
serialmeter.cc: In member function 'virtual void
SerialMeter::checkResources()':
serialmeter.cc:71: error: 'UART_LSR' was not declared in this scope
serialmeter.cc:71: error: 'UART_MSR' was not declared in this scope
..
Signed-off-by: Herton Ronaldo Krzesinski <[email protected]>
Cc: Gustavo de Nardin <[email protected]>
Cc: David Woodhouse <[email protected]>
Cc: Russell King <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Willy Tarreau <[email protected]>
---
include/linux/Kbuild | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/include/linux/Kbuild b/include/linux/Kbuild
index 157db77..199fd71 100644
--- a/include/linux/Kbuild
+++ b/include/linux/Kbuild
@@ -134,6 +134,7 @@ header-y += radeonfb.h
header-y += raw.h
header-y += resource.h
header-y += rose.h
+header-y += serial_reg.h
header-y += smbno.h
header-y += snmp.h
header-y += sockios.h
--
1.5.2.5
--
On Wed, Aug 22, 2007 at 11:39:11AM +0200, Willy Tarreau wrote:
> As reported by Gustavo de Nardin <[email protected]>, while trying to
> compile xosview (http://xosview.sourceforge.net/) with upstream kernel
> headers being used you get the following errors:
> serialmeter.cc:48:30: error: linux/serial_reg.h: No such file or directory
> serialmeter.cc: In member function 'virtual void
> SerialMeter::checkResources()':
> serialmeter.cc:71: error: 'UART_LSR' was not declared in this scope
> serialmeter.cc:71: error: 'UART_MSR' was not declared in this scope
> ..
It's probably a good thing to repeat what I said privately about this,
which is:
| It should be noted that reading the MSR or LSR of an active 8250 UART
| has side effects. In the case of MSR, it clears the bits which indicate
| that the modem status signals have changed, and clears the associated
| interrupt. This can result in loss of hardware flow control on the port.
|
| For LSR, a read clears the error status reported in that register and
| clears the interrupt associated with the error status, resulting in
| errors being missed.
|
| So, reading the LSR and MSR of an active port is Bad News(tm) and is
| something that userspace should absolutely avoid. Instead, userspace
| should open the port in non-blocking mode and use the TIOCGICOUNT
| ioctl to retrieve the various activity counters and TIOCMGET to obtain
| the current modem status line state.
|
| I would encourage xosview folk to switch to use these ioctls rather than
| reading the registers direct for the following reasons:
|
| 1. It's supported by a wide variety of serial port implementations.
| 2. Avoids the mess associated with ioperm support noted in serialmeter.cc
| revision 1.11
| 3. Avoids including kernel private kernel headers.
| 4. Avoids the buggy "autodetection" of serial IO base (which ignores the
| IO type of the port, which may be MMIO.)
| 5. Avoids side effects associated with reading the ports registers.
|
| For a tool which presumably is to assist diagnosing serial problems, it
| would seem to have the capability of causing further additional problems.
I leave it up to others to decide that keeping backwards compatibility
with userspace which is quite obviously broken is more important than
providing userspace with the persuasion to fix their broken code.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
Hi Russell,
On Wed, Aug 22, 2007 at 10:23:32AM +0100, Russell King wrote:
> On Wed, Aug 22, 2007 at 11:39:11AM +0200, Willy Tarreau wrote:
> > As reported by Gustavo de Nardin <[email protected]>, while trying to
> > compile xosview (http://xosview.sourceforge.net/) with upstream kernel
> > headers being used you get the following errors:
> > serialmeter.cc:48:30: error: linux/serial_reg.h: No such file or directory
> > serialmeter.cc: In member function 'virtual void
> > SerialMeter::checkResources()':
> > serialmeter.cc:71: error: 'UART_LSR' was not declared in this scope
> > serialmeter.cc:71: error: 'UART_MSR' was not declared in this scope
> > ..
>
> It's probably a good thing to repeat what I said privately about this,
> which is:
>
> | It should be noted that reading the MSR or LSR of an active 8250 UART
> | has side effects. In the case of MSR, it clears the bits which indicate
> | that the modem status signals have changed, and clears the associated
> | interrupt. This can result in loss of hardware flow control on the port.
(...)
> | For a tool which presumably is to assist diagnosing serial problems, it
> | would seem to have the capability of causing further additional problems.
>
> I leave it up to others to decide that keeping backwards compatibility
> with userspace which is quite obviously broken is more important than
> providing userspace with the persuasion to fix their broken code.
While I agree with you that side effects may be undesirable, I still think
it's important to ensure backwards compatibility in -stable branch, for the
following reasons :
- people who use -stable want to ensure that they can upgrade in order to
fix security issues or other bugs without risking (much) regression
- you do not prevent buggy programs from doing wrong things by removing
headers. What you will end up with is xosview using the quite common
#ifndef UART_LSR / #define UART_LSR / #endif. In fact, there *may* be
some legitimate uses of those defines, eventhough this program abuses
its capabilities.
Eventhough it's not a good practise to rely on kernel headers for userspace,
at least we should limit build regressions on the user side, as it does not
help the end user understand that the program he builds is wrong.
Thanks,
Willy