* Re: [PATCH -next 2/2] tty: Correct tty buffer flush.
From: Jiri Slaby @ 2012-12-05 9:35 UTC (permalink / raw)
To: Ilya Zykov; +Cc: Greg Kroah-Hartman, Alan Cox, linux-kernel, linux-serial
In-Reply-To: <50BF0A56.9050209@ilyx.ru>
On 12/05/2012 09:48 AM, Ilya Zykov wrote:
> tty: Correct tty buffer flush.
NAK just because of the insufficient commit log. That line does not
belong here. Instead, please add here proper description as you have
already done before. IOW what is in 0/2 should be here so that we know
the reasons. 0/2 text is not stored in git. This one is.
> Signed-off-by: Ilya Zykov <ilya@ilyx.ru>
> ---
> drivers/tty/tty_buffer.c | 11 +++++++----
> 1 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
> index 7602df8..8a3333d 100644
> --- a/drivers/tty/tty_buffer.c
> +++ b/drivers/tty/tty_buffer.c
> @@ -119,11 +119,14 @@ static void __tty_buffer_flush(struct tty_port *port)
> struct tty_bufhead *buf = &port->buf;
> struct tty_buffer *thead;
>
> - while ((thead = buf->head) != NULL) {
> - buf->head = thead->next;
> - tty_buffer_free(port, thead);
> + if (unlikely(buf->head == NULL))
> + return;
> + while ((thead = buf->head->next) != NULL) {
> + tty_buffer_free(port, buf->head);
> + buf->head = thead;
> }
> - buf->tail = NULL;
> + WARN_ON(buf->head != buf->tail);
> + buf->head->read = buf->head->commit;
> }
>
> /**
>
thanks,
--
js
suse labs
^ permalink raw reply
* [PATCH -next 2/2] tty: Correct tty buffer flush.
From: Ilya Zykov @ 2012-12-05 8:48 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Alan Cox, Jiri Slaby, linux-kernel, linux-serial
tty: Correct tty buffer flush.
Signed-off-by: Ilya Zykov <ilya@ilyx.ru>
---
drivers/tty/tty_buffer.c | 11 +++++++----
1 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 7602df8..8a3333d 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -119,11 +119,14 @@ static void __tty_buffer_flush(struct tty_port *port)
struct tty_bufhead *buf = &port->buf;
struct tty_buffer *thead;
- while ((thead = buf->head) != NULL) {
- buf->head = thead->next;
- tty_buffer_free(port, thead);
+ if (unlikely(buf->head == NULL))
+ return;
+ while ((thead = buf->head->next) != NULL) {
+ tty_buffer_free(port, buf->head);
+ buf->head = thead;
}
- buf->tail = NULL;
+ WARN_ON(buf->head != buf->tail);
+ buf->head->read = buf->head->commit;
}
/**
^ permalink raw reply related
* [PATCH -next 1/2] tty: Correct tty buffer flush.
From: Ilya Zykov @ 2012-12-05 8:48 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Alan Cox, Jiri Slaby, linux-kernel, linux-serial
Revert: tty: hold lock across tty buffer finding and buffer filling
Signed-off-by: Ilya Zykov <ilya@ilyx.ru>
---
drivers/tty/tty_buffer.c | 94 +++++++++++-----------------------------------
1 files changed, 22 insertions(+), 72 deletions(-)
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 45d9161..7602df8 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -193,20 +193,26 @@ static struct tty_buffer *tty_buffer_find(struct tty_port *port, size_t size)
/* Should possibly check if this fails for the largest buffer we
have queued and recycle that ? */
}
+
/**
- * __tty_buffer_request_room - grow tty buffer if needed
+ * tty_buffer_request_room - grow tty buffer if needed
* @tty: tty structure
* @size: size desired
*
* Make at least size bytes of linear space available for the tty
* buffer. If we fail return the size we managed to find.
- * Locking: Caller must hold port->buf.lock
+ *
+ * Locking: Takes tty->port->buf.lock
*/
-static int __tty_buffer_request_room(struct tty_port *port, size_t size)
+int tty_buffer_request_room(struct tty_struct *tty, size_t size)
{
- struct tty_bufhead *buf = &port->buf;
+ struct tty_bufhead *buf = &tty->port->buf;
struct tty_buffer *b, *n;
int left;
+ unsigned long flags;
+
+ spin_lock_irqsave(&buf->lock, flags);
+
/* OPTIMISATION: We could keep a per tty "zero" sized buffer to
remove this conditional if its worth it. This would be invisible
to the callers */
@@ -218,7 +224,7 @@ static int __tty_buffer_request_room(struct tty_port *port, size_t size)
if (left < size) {
/* This is the slow path - looking for new buffers to use */
- if ((n = tty_buffer_find(port, size)) != NULL) {
+ if ((n = tty_buffer_find(tty->port, size)) != NULL) {
if (b != NULL) {
b->next = n;
b->commit = b->used;
@@ -229,31 +235,9 @@ static int __tty_buffer_request_room(struct tty_port *port, size_t size)
size = left;
}
+ spin_unlock_irqrestore(&buf->lock, flags);
return size;
}
-
-
-/**
- * tty_buffer_request_room - grow tty buffer if needed
- * @tty: tty structure
- * @size: size desired
- *
- * Make at least size bytes of linear space available for the tty
- * buffer. If we fail return the size we managed to find.
- *
- * Locking: Takes port->buf.lock
- */
-int tty_buffer_request_room(struct tty_struct *tty, size_t size)
-{
- struct tty_port *port = tty->port;
- unsigned long flags;
- int length;
-
- spin_lock_irqsave(&port->buf.lock, flags);
- length = __tty_buffer_request_room(port, size);
- spin_unlock_irqrestore(&port->buf.lock, flags);
- return length;
-}
EXPORT_SYMBOL_GPL(tty_buffer_request_room);
/**
@@ -272,26 +256,17 @@ EXPORT_SYMBOL_GPL(tty_buffer_request_room);
int tty_insert_flip_string_fixed_flag(struct tty_struct *tty,
const unsigned char *chars, char flag, size_t size)
{
- struct tty_bufhead *buf = &tty->port->buf;
int copied = 0;
do {
int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE);
- int space;
- unsigned long flags;
- struct tty_buffer *tb;
-
- spin_lock_irqsave(&buf->lock, flags);
- space = __tty_buffer_request_room(tty->port, goal);
- tb = buf->tail;
+ int space = tty_buffer_request_room(tty, goal);
+ struct tty_buffer *tb = tty->port->buf.tail;
/* If there is no space then tb may be NULL */
- if (unlikely(space == 0)) {
- spin_unlock_irqrestore(&buf->lock, flags);
+ if (unlikely(space == 0))
break;
- }
memcpy(tb->char_buf_ptr + tb->used, chars, space);
memset(tb->flag_buf_ptr + tb->used, flag, space);
tb->used += space;
- spin_unlock_irqrestore(&buf->lock, flags);
copied += space;
chars += space;
/* There is a small chance that we need to split the data over
@@ -318,26 +293,17 @@ EXPORT_SYMBOL(tty_insert_flip_string_fixed_flag);
int tty_insert_flip_string_flags(struct tty_struct *tty,
const unsigned char *chars, const char *flags, size_t size)
{
- struct tty_bufhead *buf = &tty->port->buf;
int copied = 0;
do {
int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE);
- int space;
- unsigned long __flags;
- struct tty_buffer *tb;
-
- spin_lock_irqsave(&buf->lock, __flags);
- space = __tty_buffer_request_room(tty->port, goal);
- tb = buf->tail;
+ int space = tty_buffer_request_room(tty, goal);
+ struct tty_buffer *tb = tty->port->buf.tail;
/* If there is no space then tb may be NULL */
- if (unlikely(space == 0)) {
- spin_unlock_irqrestore(&buf->lock, __flags);
+ if (unlikely(space == 0))
break;
- }
memcpy(tb->char_buf_ptr + tb->used, chars, space);
memcpy(tb->flag_buf_ptr + tb->used, flags, space);
tb->used += space;
- spin_unlock_irqrestore(&buf->lock, __flags);
copied += space;
chars += space;
flags += space;
@@ -393,21 +359,13 @@ EXPORT_SYMBOL(tty_schedule_flip);
int tty_prepare_flip_string(struct tty_struct *tty, unsigned char **chars,
size_t size)
{
- struct tty_bufhead *buf = &tty->port->buf;
- int space;
- unsigned long flags;
- struct tty_buffer *tb;
-
- spin_lock_irqsave(&buf->lock, flags);
- space = __tty_buffer_request_room(tty->port, size);
-
- tb = buf->tail;
+ int space = tty_buffer_request_room(tty, size);
if (likely(space)) {
+ struct tty_buffer *tb = tty->port->buf.tail;
*chars = tb->char_buf_ptr + tb->used;
memset(tb->flag_buf_ptr + tb->used, TTY_NORMAL, space);
tb->used += space;
}
- spin_unlock_irqrestore(&buf->lock, flags);
return space;
}
EXPORT_SYMBOL_GPL(tty_prepare_flip_string);
@@ -431,21 +389,13 @@ EXPORT_SYMBOL_GPL(tty_prepare_flip_string);
int tty_prepare_flip_string_flags(struct tty_struct *tty,
unsigned char **chars, char **flags, size_t size)
{
- struct tty_bufhead *buf = &tty->port->buf;
- int space;
- unsigned long __flags;
- struct tty_buffer *tb;
-
- spin_lock_irqsave(&buf->lock, __flags);
- space = __tty_buffer_request_room(tty->port, size);
-
- tb = buf->tail;
+ int space = tty_buffer_request_room(tty, size);
if (likely(space)) {
+ struct tty_buffer *tb = tty->port->buf.tail;
*chars = tb->char_buf_ptr + tb->used;
*flags = tb->flag_buf_ptr + tb->used;
tb->used += space;
}
- spin_unlock_irqrestore(&buf->lock, __flags);
return space;
}
EXPORT_SYMBOL_GPL(tty_prepare_flip_string_flags);
^ permalink raw reply related
* [PATCH -next 0/2] tty: Correct tty buffer flush.
From: Ilya Zykov @ 2012-12-05 8:47 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Alan Cox, Jiri Slaby, linux-kernel, linux-serial
The root of problem is carelessly zeroing pointer(in function __tty_buffer_flush()),
when another thread can use it. It can be cause of "NULL pointer dereference".
Main idea of the patch, this is never free last (struct tty_buffer) in the active buffer.
Only flush the data for ldisc(tty->buf.head->read = tty->buf.head->commit).
At that moment driver can collect(write) data in buffer without conflict.
It is repeat behavior of flush_to_ldisc(), only without feeding data to ldisc.
Also revert:
commit c56a00a165712fd73081f40044b1e64407bb1875
tty: hold lock across tty buffer finding and buffer filling
In order to delete the unneeded locks any more.
Signed-off-by: Ilya Zykov <ilya@ilyx.ru>
^ permalink raw reply
* Re: [RFC PATCH 1/3] UART: Add UART subsystem as a bus.
From: Mika Westerberg @ 2012-12-05 7:42 UTC (permalink / raw)
To: Zheng, Lv
Cc: Alan Cox, Brown, Len, Wysocki, Rafael J, Greg Kroah-Hartman,
linux-acpi@vger.kernel.org, linux-serial@vger.kernel.org
In-Reply-To: <1AE640813FDE7649BE1B193DEA596E88BD6A54@SHSMSX101.ccr.corp.intel.com>
On Wed, Dec 05, 2012 at 07:07:52AM +0000, Zheng, Lv wrote:
> > On Tue, Dec 04, 2012 at 07:50:30PM +0000, Alan Cox wrote:
> > > > And if we have enumerated the UART controller from ACPI (it is
> > > > probably attached to the platform bus) we can find the tty device it
> > > > exports like:
> > >
> > > The property should not be in any ACPI specific form or space - just
> > > attach it directly to the tty from ACPI, DT, driver internal
> > > knowledge, PCI id, whatever
> >
> > The only property that comes into mind is _HID/_CID (referring to the ACPI
> > ID) that can be used by userspace to find out type of the device behind the
> > UART port. I don't know what name would be generic enough for the property,
> > though.
> >
> > There are other resources as well in addition to the UartSerialBus(). For
> > example we might have two GPIO lines connected to the bluetooth chip and
> > these are represented as GpioIo ACPI resources.
> >
> > Since the bluetooth is mostly handled by the N_HCI line discipline, should the
> > GPIO handling be done there as well? It can distinguish between DT and ACPI
> > enumerated devices by comparing dev->of_node and ACPI_HANDLE(dev) so it
> > can get the resources from both DT and ACPI but I'm not sure if it really
> > belongs there. Or should this be in a separate driver?
>
> IMO,
> For ACPI enumerated target devices, ACPI can provide GPIO enumeration API
> by feeding ACPI_HANDLE(tty->target) to obtain the GPIO resources while OF
> can offer its own implementation.
> Then there are 2 possible solutions can be found by calling such APIs:
> 1. implement GPIO enabling in the kernel side N_HCI proto driver.
> 2. implement GPIO enabling in the kernel side UART driver on TIOCSETD.
OK.
> Same issues can be found for the ACPI enumerated SPI/I2C target devices.
> Thus the GpioIrq and GpioIo is not handled in this patch set.
Yeah, I wasn't expecting that this series addresses that :-) However, this
is something we need to solve at some point - we probably don't want that
userspace deals with the GPIOs.
^ permalink raw reply
* RE: [RFC PATCH 1/3] UART: Add UART subsystem as a bus.
From: Zheng, Lv @ 2012-12-05 7:07 UTC (permalink / raw)
To: Mika Westerberg, Alan Cox
Cc: Brown, Len, Wysocki, Rafael J, Greg Kroah-Hartman,
linux-acpi@vger.kernel.org, linux-serial@vger.kernel.org
In-Reply-To: <20121205062015.GR3117@intel.com>
> On Tue, Dec 04, 2012 at 07:50:30PM +0000, Alan Cox wrote:
> > > And if we have enumerated the UART controller from ACPI (it is
> > > probably attached to the platform bus) we can find the tty device it
> > > exports like:
> >
> > The property should not be in any ACPI specific form or space - just
> > attach it directly to the tty from ACPI, DT, driver internal
> > knowledge, PCI id, whatever
>
> The only property that comes into mind is _HID/_CID (referring to the ACPI
> ID) that can be used by userspace to find out type of the device behind the
> UART port. I don't know what name would be generic enough for the property,
> though.
>
> There are other resources as well in addition to the UartSerialBus(). For
> example we might have two GPIO lines connected to the bluetooth chip and
> these are represented as GpioIo ACPI resources.
>
> Since the bluetooth is mostly handled by the N_HCI line discipline, should the
> GPIO handling be done there as well? It can distinguish between DT and ACPI
> enumerated devices by comparing dev->of_node and ACPI_HANDLE(dev) so it
> can get the resources from both DT and ACPI but I'm not sure if it really
> belongs there. Or should this be in a separate driver?
IMO,
For ACPI enumerated target devices, ACPI can provide GPIO enumeration API by feeding ACPI_HANDLE(tty->target) to obtain the GPIO resources while OF can offer its own implementation.
Then there are 2 possible solutions can be found by calling such APIs:
1. implement GPIO enabling in the kernel side N_HCI proto driver.
2. implement GPIO enabling in the kernel side UART driver on TIOCSETD.
Same issues can be found for the ACPI enumerated SPI/I2C target devices.
Thus the GpioIrq and GpioIo is not handled in this patch set.
Best regards/Lv Zheng
^ permalink raw reply
* Re: [RFC PATCH 1/3] UART: Add UART subsystem as a bus.
From: Mika Westerberg @ 2012-12-05 6:20 UTC (permalink / raw)
To: Alan Cox
Cc: Lv Zheng, Len Brown, Rafael J Wysocki, Greg Kroah-Hartman,
linux-acpi, linux-serial
In-Reply-To: <20121204195030.238d1b71@bob.linux.org.uk>
On Tue, Dec 04, 2012 at 07:50:30PM +0000, Alan Cox wrote:
> > And if we have enumerated the UART controller from ACPI (it is
> > probably attached to the platform bus) we can find the tty device it
> > exports like:
>
> The property should not be in any ACPI specific form or space - just
> attach it directly to the tty from ACPI, DT, driver internal knowledge,
> PCI id, whatever
The only property that comes into mind is _HID/_CID (referring to the ACPI
ID) that can be used by userspace to find out type of the device behind the
UART port. I don't know what name would be generic enough for the property,
though.
There are other resources as well in addition to the UartSerialBus(). For
example we might have two GPIO lines connected to the bluetooth chip and
these are represented as GpioIo ACPI resources.
Since the bluetooth is mostly handled by the N_HCI line discipline, should
the GPIO handling be done there as well? It can distinguish between DT and
ACPI enumerated devices by comparing dev->of_node and ACPI_HANDLE(dev) so
it can get the resources from both DT and ACPI but I'm not sure if it
really belongs there. Or should this be in a separate driver?
^ permalink raw reply
* [PATCH v2 4/4] UART: Add dummy devices to test the enumeration.
From: Lv Zheng @ 2012-12-05 3:52 UTC (permalink / raw)
To: Len Brown, Rafael J Wysocki, Greg Kroah-Hartman, Alan Cox,
Mika Westerberg
Cc: linux-acpi, linux-serial, Lv Zheng
In-Reply-To: <cover.1354677952.git.lv.zheng@intel.com>
This is a test patch that should not be merged to any of the published
Linux source tree.
1. The result of the UART dummy target device is as follows:
# udevadm monitor --kernel --environment > ~/uart.uevents
# echo add > /sys/bus/uart/uevent
# echo add > /sys/bus/uart/devices/DUMMY/uevent
# cat ~/uart.uevents
monitor will print the received events for:
KERNEL - the kernel uevent
KERNEL[252.443458] add /bus/uart (bus)
ACTION=add
DEVPATH=/bus/uart
SEQNUM=1142
SUBSYSTEM=bus
KERNEL[268.491709] add /devices/platform/serial8250/DUMMY (uart)
ACTION=add
DEVPATH=/devices/platform/serial8250/DUMMY
DEVTYPE=uart_device
MODALIAS=uart:DUMMY
SEQNUM=1144
SUBSYSTEM=uart
# cat /sys/bus/uart/devices/DUMMY/modalias
uart:DUMMY
# cat /sys/bus/uart/devices/DUMMY/tty_dev
ttyS3
# cat /sys/bus/uart/devices/DUMMY/tty_attrs
115200 8N1 HW SW
# cat /sys/bus/uart/devices/DUMMY/modem_lines
LE:RTS,
# ls -l /sys/bus/uart/devices
DUMMY -> ../../../devices/platform/serial8250/DUMMY
# ls -l /sys/devices/platform/serial8250/DUMMY
subsystem -> ../../../../bus/uart
host_node -> ../tty/ttyS3
# ls -l /sys/devices/platform/serial8250/tty/ttyS3
target_node -> ../../DUMMY
2. The result of the UART customized DSDT target device is as follows:
The test is based on the following customized DSDT (containing the dummy
uart host adapter INTF000 and target device INTF001):
Device (UA00)
{
Name (_HID, "INTF000") // _HID: Hardware ID
Name (RBUF, ResourceTemplate ()
{
Memory32Fixed (ReadWrite,
0x00000000, // Address Base
0x00001000) // Address Length
})
Method (_CRS, 0, NotSerialized) // _CRS: Current Resource Settings
{
Return (RBUF)
}
Method (_STA, 0, NotSerialized) // _STA: Status
{
Return (0x0F)
}
Device (BTH0)
{
Name (_HID, "INTF001") // _HID: Hardware ID
Method (_CRS, 0, NotSerialized) // _CRS: Current Resource Settings
{
Name (UBUF, ResourceTemplate ()
{
UartSerialBus (0x0001C200, DataBitsEight, StopBitsOne,
0xC0, LittleEndian, ParityTypeNone, FlowControlHardware,
0x0020, 0x0020, "\\_SB.PCI0.UA00",
0x00, ResourceConsumer, ,
)
})
Return (UBUF)
}
Method (_STA, 0, NotSerialized) // _STA: Status
{
Return (0x0F)
}
}
}
# udevadm monitor --kernel --environment > ~/uart.uevents
# echo add > /sys/bus/uart/uevent
# echo add > /sys/bus/uart/devices/INTF001/uevent
# cat ~/uart.uevents
monitor will print the received events for:
KERNEL - the kernel uevent
KERNEL[252.443458] add /bus/uart (bus)
ACTION=add
DEVPATH=/bus/uart
SEQNUM=1142
SUBSYSTEM=bus
KERNEL[268.491709] add /devices/platform/INTF000:00/INTF001:00 (uart)
ACTION=add
DEVPATH=/devices/platform/INTF000:00/INTF001:00
DEVTYPE=uart_device
MODALIAS=uart:INTF001:00
SEQNUM=1144
SUBSYSTEM=uart
# cat /sys/bus/uart/devices/INTF001:00/modalias
uart:INTF001:00
# cat /sys/bus/uart/devices/INTF001:00/tty_dev
ttyS0
# cat /sys/bus/uart/devices/INTF001:00/tty_attrs
115200 8N0 HW
# cat /sys/bus/uart/devices/INTF001:00/modem_lines
LE:RTS,CTS,
# ls -l /sys/bus/uart/devices
INTF001:00 -> ../../../devices/platform/INTF000:00/INTF001:00
# ls -l /sys/devices/platform/INTF000:00/INTF001:00
subsystem -> ../../../../bus/uart
host_node -> ../tty/ttyS0
# ls -l /sys/devices/platform/INTF000:00/tty/ttyS0
target_node -> ../../INTF001:00
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
drivers/acpi/scan.c | 1 +
drivers/tty/serial/8250/8250.c | 12 ++++
drivers/tty/serial/8250/8250_dummy.c | 129 ++++++++++++++++++++++++++++++++++
drivers/tty/serial/8250/Kconfig | 10 +++
drivers/tty/serial/8250/Makefile | 1 +
drivers/tty/serial/serial_bus.c | 17 +++++
include/linux/serial_bus.h | 5 ++
7 files changed, 175 insertions(+)
create mode 100644 drivers/tty/serial/8250/8250_dummy.c
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 67a7fa6..4892015 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -36,6 +36,7 @@ static const char *dummy_hid = "device";
static const struct acpi_device_id acpi_platform_device_ids[] = {
{ "PNP0D40" },
+ { "INTF000" },
{ }
};
diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
index af78374..745e05d 100644
--- a/drivers/tty/serial/8250/8250.c
+++ b/drivers/tty/serial/8250/8250.c
@@ -35,6 +35,7 @@
#include <linux/serial_core.h>
#include <linux/serial.h>
#include <linux/serial_8250.h>
+#include <linux/serial_bus.h>
#include <linux/nmi.h>
#include <linux/mutex.h>
#include <linux/slab.h>
@@ -2986,6 +2987,8 @@ void serial8250_resume_port(int line)
uart_resume_port(&serial8250_reg, port);
}
+struct uart_device *uart_dummy;
+
/*
* Register a set of serial devices attached to a platform device. The
* list is terminated with a zero flags entry, which means we expect
@@ -2996,6 +2999,7 @@ static int __devinit serial8250_probe(struct platform_device *dev)
struct plat_serial8250_port *p = dev->dev.platform_data;
struct uart_8250_port uart;
int ret, i, irqflag = 0;
+ unsigned int dummy_line;
memset(&uart, 0, sizeof(uart));
@@ -3031,6 +3035,13 @@ static int __devinit serial8250_probe(struct platform_device *dev)
p->irq, ret);
}
}
+
+ dummy_line = serial8250_ports[nr_uarts-1].port.line;
+ pr_info("registering DUMMY at line %d.\n", dummy_line);
+ uart_dummy = uart_register_dummy(&dev->dev,
+ serial8250_reg.tty_driver,
+ dummy_line);
+
return 0;
}
@@ -3041,6 +3052,7 @@ static int __devexit serial8250_remove(struct platform_device *dev)
{
int i;
+ uart_unregister_device(uart_dummy);
for (i = 0; i < nr_uarts; i++) {
struct uart_8250_port *up = &serial8250_ports[i];
diff --git a/drivers/tty/serial/8250/8250_dummy.c b/drivers/tty/serial/8250/8250_dummy.c
new file mode 100644
index 0000000..c5ec064
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_dummy.c
@@ -0,0 +1,129 @@
+/*
+ * 8250_dummy.c: Dummy 8250 UART target device enumerator
+ *
+ * Copyright (c) 2012, Intel Corporation
+ * Author: Lv Zheng <lv.zheng@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ */
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/serial_8250.h>
+#include <linux/serial_bus.h>
+#include <linux/acpi.h>
+#include <linux/acpi_uart.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+struct dummy8250_data {
+ int last_lcr;
+ int line;
+};
+
+static void dummy8250_serial_out(struct uart_port *p, int offset, int value)
+{
+}
+
+static unsigned int dummy8250_serial_in(struct uart_port *p, int offset)
+{
+ return 0;
+}
+
+struct klist *dummy8250_mgr;
+int dummy8250_num;
+
+static int __devinit dummy8250_probe(struct platform_device *pdev)
+{
+ struct uart_8250_port uart = {};
+ struct resource *regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ struct dummy8250_data *data;
+#ifdef CONFIG_ACPI_UART
+ struct klist *mgr;
+#endif
+
+ dev_info(&pdev->dev, "1\n");
+ if (!regs) {
+ dev_err(&pdev->dev, "no registers defined\n");
+ return -EINVAL;
+ }
+
+ dev_info(&pdev->dev, "2\n");
+ data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+ uart.port.private_data = data;
+
+ spin_lock_init(&uart.port.lock);
+ uart.port.mapbase = regs->start;
+ uart.port.type = PORT_8250;
+ uart.port.flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF | UPF_IOREMAP |
+ UPF_FIXED_PORT | UPF_FIXED_TYPE;
+ uart.port.dev = &pdev->dev;
+
+ uart.port.iotype = UPIO_MEM;
+ uart.port.serial_in = dummy8250_serial_in;
+ uart.port.serial_out = dummy8250_serial_out;
+ uart.port.uartclk = 40000000;
+
+ dev_info(&pdev->dev, "3\n");
+ data->line = serial8250_register_8250_port(&uart);
+ if (data->line < 0)
+ return data->line;
+
+#ifdef CONFIG_ACPI_UART
+ dev_info(&pdev->dev, "4\n");
+ mgr = acpi_uart_register_devices(dummy8250_mgr,
+ &pdev->dev,
+ serial8250_reg.tty_driver,
+ data->line);
+ if (mgr) {
+ dummy8250_mgr = mgr;
+ dummy8250_num++;
+ }
+#endif
+ dev_info(&pdev->dev, "5\n");
+ platform_set_drvdata(pdev, data);
+
+ return 0;
+}
+
+static int __devexit dummy8250_remove(struct platform_device *pdev)
+{
+ struct dummy8250_data *data = platform_get_drvdata(pdev);
+
+#ifdef CONFIG_ACPI_UART
+ dummy8250_num--;
+ if (!dummy8250_num)
+ acpi_uart_unregister_devices(dummy8250_mgr);
+#endif
+ serial8250_unregister_port(data->line);
+
+ return 0;
+}
+
+static const struct acpi_device_id dummy8250_match[] = {
+ { .id = "INTF000" },
+ { /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(acpi, dummy8250_match);
+
+static struct platform_driver dummy8250_platform_driver = {
+ .driver = {
+ .name = "dummy-uart",
+ .owner = THIS_MODULE,
+ .acpi_match_table = dummy8250_match,
+ },
+ .probe = dummy8250_probe,
+ .remove = __devexit_p(dummy8250_remove),
+};
+
+module_platform_driver(dummy8250_platform_driver);
+
+MODULE_AUTHOR("Lv Zheng");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Dummy 8250 serial port driver");
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index f3d283f..3ba480a 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -270,6 +270,16 @@ config SERIAL_8250_DW
Selecting this option will enable handling of the extra features
present in the Synopsys DesignWare APB UART.
+config SERIAL_8250_DUMMY
+ tristate "Support for dummy ACPI 8250"
+ depends on SERIAL_8250 && ACPI
+ help
+ Selecting this option will enable a test UART target device DUMMY
+ under the ISA serial8250 and a test UART host adapter INTF000
+ as an platform device for the purpose of testing the ACPI UART
+ enumeration support.
+ If unsure, say "N" here.
+
config SERIAL_8250_EM
tristate "Support for Emma Mobile intergrated serial port"
depends on SERIAL_8250 && ARM && HAVE_CLK
diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
index 108fe7f..fb82aa9 100644
--- a/drivers/tty/serial/8250/Makefile
+++ b/drivers/tty/serial/8250/Makefile
@@ -19,3 +19,4 @@ obj-$(CONFIG_SERIAL_8250_HUB6) += 8250_hub6.o
obj-$(CONFIG_SERIAL_8250_FSL) += 8250_fsl.o
obj-$(CONFIG_SERIAL_8250_DW) += 8250_dw.o
obj-$(CONFIG_SERIAL_8250_EM) += 8250_em.o
+obj-$(CONFIG_SERIAL_8250_DUMMY) += 8250_dummy.o
diff --git a/drivers/tty/serial/serial_bus.c b/drivers/tty/serial/serial_bus.c
index afbb885..d25058a 100644
--- a/drivers/tty/serial/serial_bus.c
+++ b/drivers/tty/serial/serial_bus.c
@@ -387,6 +387,23 @@ void uart_del_adapter(struct klist *adap)
}
EXPORT_SYMBOL_GPL(uart_del_adapter);
+static struct uart_board_info dummy_target = {
+ .type = "DUMMY",
+ .baud = 115200,
+ .cflag = CS8 | CSTOPB | CRTSCTS,
+ .iflag = (IXON | IXOFF),
+ .mctrl = TIOCM_LE | TIOCM_RTS,
+};
+
+struct uart_device *uart_register_dummy(struct device *dev,
+ struct tty_driver *drv,
+ unsigned int line)
+{
+ dummy_target.line = line;
+ return uart_register_device(NULL, dev, drv, &dummy_target);
+}
+EXPORT_SYMBOL_GPL(uart_register_dummy);
+
struct bus_type uart_bus_type = {
.name = "uart",
};
diff --git a/include/linux/serial_bus.h b/include/linux/serial_bus.h
index 6d66fe2..917bbae 100644
--- a/include/linux/serial_bus.h
+++ b/include/linux/serial_bus.h
@@ -72,4 +72,9 @@ struct device *uart_tty_find(struct tty_driver *drv, unsigned int line,
struct device *dev);
void uart_tty_name(struct tty_driver *driver, unsigned int line, char *p);
+/* Test dummy device registration */
+struct uart_device *uart_register_dummy(struct device *dev,
+ struct tty_driver *drv,
+ unsigned int line);
+
#endif /* LINUX_SERIAL_BUS_H */
--
1.7.10
^ permalink raw reply related
* [PATCH v2 3/4] UART / 8250: Add declearation of serial8250 driver.
From: Lv Zheng @ 2012-12-05 3:51 UTC (permalink / raw)
To: Len Brown, Rafael J Wysocki, Greg Kroah-Hartman, Alan Cox,
Mika Westerberg
Cc: linux-acpi, linux-serial, Lv Zheng
In-Reply-To: <cover.1354677952.git.lv.zheng@intel.com>
Export serial8250 uart driver for uart bus enumerator users.
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
drivers/tty/serial/8250/8250.c | 5 ++---
include/linux/serial_8250.h | 2 ++
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
index 3ba4234..af78374 100644
--- a/drivers/tty/serial/8250/8250.c
+++ b/drivers/tty/serial/8250/8250.c
@@ -56,8 +56,6 @@ static unsigned int share_irqs = SERIAL8250_SHARE_IRQS;
static unsigned int nr_uarts = CONFIG_SERIAL_8250_RUNTIME_UARTS;
-static struct uart_driver serial8250_reg;
-
static int serial_index(struct uart_port *port)
{
return (serial8250_reg.minor - 64) + port->line;
@@ -2902,7 +2900,7 @@ int serial8250_find_port(struct uart_port *p)
#define SERIAL8250_CONSOLE NULL
#endif
-static struct uart_driver serial8250_reg = {
+struct uart_driver serial8250_reg = {
.owner = THIS_MODULE,
.driver_name = "serial",
.dev_name = "ttyS",
@@ -2910,6 +2908,7 @@ static struct uart_driver serial8250_reg = {
.minor = 64,
.cons = SERIAL8250_CONSOLE,
};
+EXPORT_SYMBOL_GPL(serial8250_reg);
/*
* early_serial_setup - early registration for 8250 ports
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index c174c90..687af38 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -96,6 +96,8 @@ struct uart_8250_port {
void (*dl_write)(struct uart_8250_port *, int);
};
+extern struct uart_driver serial8250_reg;
+
int serial8250_register_8250_port(struct uart_8250_port *);
void serial8250_unregister_port(int line);
void serial8250_suspend_port(int line);
--
1.7.10
^ permalink raw reply related
* [PATCH v2 2/4] ACPI / UART: Add ACPI enumeration support for UART bus.
From: Lv Zheng @ 2012-12-05 3:51 UTC (permalink / raw)
To: Len Brown, Rafael J Wysocki, Greg Kroah-Hartman, Alan Cox,
Mika Westerberg
Cc: linux-acpi, linux-serial, Lv Zheng
In-Reply-To: <cover.1354677952.git.lv.zheng@intel.com>
ACPI 5.0 specification introduces the methods of enumerating the target
devices connected on the serial buses.
This patch follows the specification, implementing such UART enumeration
machanism for the Linux.
In order to use this UART device enumeration mechanism, driver writers
are required to call the following APIs:
1. APIs called _after_ the creation of the uart ports:
adap = acpi_uart_register_devices(driver, adap, parent, line);
Where:
driver: the low level UART driver
parent: the physical device of the UART ports
adap: the management list of the target devices, can be set as NULL
line: the line number of the target device
2. APIs called _before_ the deletion of the uart ports:
acpi_uart_unregister_devices(adap);
Where:
adap: the management list of the UART target devices
NOTE: If the driver writer has already created the management list for
the UART target devices, the adap parameter can be set to the
already created non-NULL value.
NOTE: The ACPI 5.0 specification assumes one physical device per-port.
In this situation, the line parameter might be set to 0 when the
acpi_uart_register_devices() is called.
This patch set can also support the multi-port UART adapters,
where the line should be set as ACPI_UART_LINE_UNKNOWN.
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
drivers/acpi/Kconfig | 7 ++
drivers/acpi/Makefile | 1 +
drivers/acpi/acpi_uart.c | 262 +++++++++++++++++++++++++++++++++++++++++++++
include/linux/acpi_uart.h | 40 +++++++
4 files changed, 310 insertions(+)
create mode 100644 drivers/acpi/acpi_uart.c
create mode 100644 include/linux/acpi_uart.h
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 0300bf6..d40203f 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -187,6 +187,12 @@ config ACPI_I2C
help
ACPI I2C enumeration support.
+config ACPI_UART
+ def_tristate SERIAL_CORE
+ depends on SERIAL_CORE
+ help
+ ACPI UART enumeration support.
+
config ACPI_PROCESSOR
tristate "Processor"
select THERMAL
@@ -200,6 +206,7 @@ config ACPI_PROCESSOR
To compile this driver as a module, choose M here:
the module will be called processor.
+
config ACPI_IPMI
tristate "IPMI"
depends on EXPERIMENTAL && IPMI_SI && IPMI_HANDLER
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 2a4502b..784f332 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -71,6 +71,7 @@ obj-$(CONFIG_ACPI_EC_DEBUGFS) += ec_sys.o
obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o
obj-$(CONFIG_ACPI_BGRT) += bgrt.o
obj-$(CONFIG_ACPI_I2C) += acpi_i2c.o
+obj-$(CONFIG_ACPI_UART) += acpi_uart.o
# processor has its own "processor." module_param namespace
processor-y := processor_driver.o processor_throttling.o
diff --git a/drivers/acpi/acpi_uart.c b/drivers/acpi/acpi_uart.c
new file mode 100644
index 0000000..74caf9c
--- /dev/null
+++ b/drivers/acpi/acpi_uart.c
@@ -0,0 +1,262 @@
+/*
+ * acpi_uart.c - ACPI UART enumeration support
+ *
+ * Copyright (c) 2012, Intel Corporation
+ * Author: Lv Zheng <lv.zheng@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ */
+
+#include <linux/init.h>
+#include <linux/serial_bus.h>
+#include <linux/module.h>
+#include <linux/acpi.h>
+#include <linux/acpi_uart.h>
+#include <acpi/acpi.h>
+#include <acpi/acpi_bus.h>
+
+
+static int
+acpi_uart_add_resources(struct acpi_resource *ares, void *context)
+{
+ struct uart_board_info *info = context;
+
+ if (ares->type == ACPI_RESOURCE_TYPE_SERIAL_BUS) {
+ struct acpi_resource_uart_serialbus *sb;
+
+ sb = &ares->data.uart_serial_bus;
+ if (sb->type != ACPI_RESOURCE_SERIAL_TYPE_UART)
+ return 1;
+
+ /* baud rate */
+ info->baud = sb->default_baud_rate;
+
+ /* data bits */
+ info->cflag &= ~CSIZE;
+ switch (sb->data_bits) {
+ case ACPI_UART_5_DATA_BITS:
+ info->cflag |= CS5;
+ break;
+ case ACPI_UART_6_DATA_BITS:
+ info->cflag |= CS6;
+ break;
+ case ACPI_UART_7_DATA_BITS:
+ info->cflag |= CS7;
+ break;
+ case ACPI_UART_8_DATA_BITS:
+ default:
+ info->cflag |= CS8;
+ break;
+ }
+
+ /* parity */
+ info->cflag &= ~(PARENB | PARODD);
+ if (sb->parity == ACPI_UART_PARITY_EVEN)
+ info->cflag |= PARENB;
+ else if (sb->parity == ACPI_UART_PARITY_ODD)
+ info->cflag |= (PARENB | PARODD);
+
+ /* stop bits */
+ if (sb->stop_bits == ACPI_UART_2_STOP_BITS)
+ info->cflag |= CSTOPB;
+ else
+ info->cflag &= ~CSTOPB;
+
+ /* HW control */
+ if (sb->flow_control & ACPI_UART_FLOW_CONTROL_HW)
+ info->cflag |= CRTSCTS;
+ else
+ info->cflag &= ~CRTSCTS;
+
+ /* SW control */
+ if (sb->flow_control & ACPI_UART_FLOW_CONTROL_XON_XOFF)
+ info->iflag |= (IXON | IXOFF);
+ else
+ info->iflag &= ~(IXON|IXOFF|IXANY);
+
+ /* endianess */
+ if (sb->endian == ACPI_UART_LITTLE_ENDIAN)
+ info->mctrl |= TIOCM_LE;
+ else
+ info->mctrl &= ~TIOCM_LE;
+
+ /* terminal lines */
+ if (sb->lines_enabled & ACPI_UART_DATA_TERMINAL_READY)
+ info->mctrl |= TIOCM_DTR;
+ else
+ info->mctrl &= ~TIOCM_DTR;
+ if (sb->lines_enabled & ACPI_UART_REQUEST_TO_SEND)
+ info->mctrl |= TIOCM_RTS;
+ else
+ info->mctrl &= ~TIOCM_RTS;
+
+ /* modem lines */
+ if (sb->lines_enabled & ACPI_UART_CLEAR_TO_SEND)
+ info->mctrl |= TIOCM_CTS;
+ else
+ info->mctrl &= ~TIOCM_CTS;
+ if (sb->lines_enabled & ACPI_UART_CARRIER_DETECT)
+ info->mctrl |= TIOCM_CAR;
+ else
+ info->mctrl &= ~TIOCM_CAR;
+ if (sb->lines_enabled & ACPI_UART_RING_INDICATOR)
+ info->mctrl |= TIOCM_RNG;
+ else
+ info->mctrl &= ~TIOCM_RNG;
+ if (sb->lines_enabled & ACPI_UART_DATA_SET_READY)
+ info->mctrl |= TIOCM_DSR;
+ else
+ info->mctrl &= ~TIOCM_DSR;
+ } else if (info->irq < 0) {
+ struct resource r;
+
+ if (acpi_dev_resource_interrupt(ares, 0, &r))
+ info->irq = r.start;
+ }
+
+ return 1;
+}
+
+struct acpi_uart_walk {
+ struct tty_driver *drv;
+ unsigned int line;
+ struct device *parent;
+};
+
+static acpi_status acpi_uart_add_device(acpi_handle handle, u32 level,
+ void *context, void **return_value)
+{
+ struct acpi_uart_walk *walk = context;
+ struct tty_driver *drv = walk->drv;
+ unsigned int line = walk->line;
+ struct device *parent = walk->parent;
+ struct acpi_device_info *info;
+ struct uart_board_info board_info;
+ struct acpi_device *adev;
+ struct list_head resource_list;
+ int ret;
+ acpi_status status;
+ struct klist *adap = *return_value;
+ struct uart_device *udev;
+
+ BUG_ON(!parent || !adap || !drv);
+
+ /*
+ * Current BIOS will create physical adapter DEVICE per port.
+ * This implementation assumes multi-port DEVICE will use _ADR
+ * as serial port line number though there is no such explicit
+ * proof for this assumption in the ACPI specification.
+ *
+ * NOTE: There is no real BIOS implementation using _ADR as line
+ * number.
+ */
+ if (line == ACPI_UART_LINE_UNKNOWN) {
+ status = acpi_get_object_info(handle, &info);
+ if (ACPI_FAILURE(status))
+ return AE_OK;
+ if (info->valid & ACPI_VALID_ADR)
+ line = info->address;
+ }
+
+ if (line == ACPI_UART_LINE_UNKNOWN)
+ return AE_OK;
+ if (acpi_bus_get_device(handle, &adev))
+ return AE_OK;
+ if (acpi_bus_get_status(adev) || !adev->status.present)
+ return AE_OK;
+
+ memset(&board_info, 0, sizeof(board_info));
+ board_info.irq = -1;
+ board_info.line = line;
+
+ INIT_LIST_HEAD(&resource_list);
+ ret = acpi_dev_get_resources(adev, &resource_list,
+ acpi_uart_add_resources, &board_info);
+ acpi_dev_free_resource_list(&resource_list);
+
+ if (ret < 0 || !board_info.baud)
+ return AE_OK;
+
+ strlcpy(board_info.type, dev_name(&adev->dev), sizeof(board_info.type));
+
+ udev = uart_register_device(adap, parent, drv, &board_info);
+ if (!udev) {
+ dev_err(&adev->dev, "failed to add #%d uart device from ACPI\n",
+ line);
+ return AE_OK;
+ }
+
+ return AE_OK;
+}
+
+/**
+ * acpi_uart_register_devices - register the uart slave devices behind the
+ * uart host adapter
+ * @adap: the logical uart host adapter
+ * @dev: the physical uart host adapter
+ * @drv: the tty host driver
+ * @line: the serial line number
+ *
+ * Enumerate all uart slave devices behind the adapter by walking the ACPI
+ * namespace. When a device is found, it will be added to the Linux device
+ * model and bound to the corresponding ACPI handle.
+ * If the physical device is a multiple port device, line should be -1.
+ * If the physical device is a single port device, line should be the line
+ * number.
+ */
+struct klist *acpi_uart_register_devices(struct klist *adap,
+ struct device *parent,
+ struct tty_driver *drv,
+ unsigned int line)
+{
+ acpi_handle handle;
+ acpi_status status;
+ struct klist *klist = NULL;
+ struct acpi_uart_walk walk = {drv, line, parent};
+
+ BUG_ON(!drv || !parent);
+
+ handle = ACPI_HANDLE(parent);
+ if (!handle)
+ return NULL;
+
+ if (!adap) {
+ klist = kzalloc(sizeof(struct klist), GFP_KERNEL);
+ if (!klist)
+ return NULL;
+ (void)uart_add_adapter(klist);
+ adap = klist;
+ }
+
+ status = acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, 1,
+ acpi_uart_add_device, NULL, &walk,
+ (void **)&adap);
+ if (ACPI_FAILURE(status)) {
+ dev_warn(parent, "failed to enumerate UART slaves\n");
+ goto fail;
+ }
+
+ return adap;
+
+fail:
+ uart_del_adapter(klist);
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(acpi_uart_register_devices);
+
+/**
+ * acpi_uart_register_devices - unregister the uart slave devices behind the
+ * uart host adapter
+ * @adap: the logical uart host adapter
+ *
+ * Reverse effect of acpi_uart_register_devices().
+ */
+void acpi_uart_unregister_devices(struct klist *adap)
+{
+ uart_del_adapter(adap);
+ kfree(adap);
+}
+EXPORT_SYMBOL_GPL(acpi_uart_unregister_devices);
diff --git a/include/linux/acpi_uart.h b/include/linux/acpi_uart.h
new file mode 100644
index 0000000..a928425
--- /dev/null
+++ b/include/linux/acpi_uart.h
@@ -0,0 +1,40 @@
+/*
+ * acpi_uart.h - ACPI UART enumeration support
+ *
+ * Copyright (c) 2012, Intel Corporation
+ * Author: Lv Zheng <lv.zheng@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ */
+
+#ifndef _LINUX_ACPI_UART_H
+#define _LINUX_ACPI_UART_H
+
+struct uart_driver;
+
+#define ACPI_UART_LINE_UNKNOWN ((unsigned int)-1)
+
+#if IS_ENABLED(CONFIG_ACPI_UART)
+struct klist *acpi_uart_register_devices(struct klist *adap,
+ struct device *parent,
+ struct tty_driver *drv,
+ unsigned int line);
+void acpi_uart_unregister_devices(struct klist *adap);
+#else
+static inline struct klist *acpi_uart_register_devices(struct klist *adap,
+ struct device *parent,
+ struct tty_driver *drv,
+ unsigned int line)
+{
+ return NULL;
+}
+
+static inline void acpi_uart_unregister_devices(struct klist *adap)
+{
+}
+#endif
+
+#endif /* _LINUX_ACPI_UART_H */
--
1.7.10
^ permalink raw reply related
* [PATCH v2 1/4] UART: Add UART subsystem as a bus.
From: Lv Zheng @ 2012-12-05 3:51 UTC (permalink / raw)
To: Len Brown, Rafael J Wysocki, Greg Kroah-Hartman, Alan Cox,
Mika Westerberg
Cc: linux-acpi, linux-serial, Lv Zheng
In-Reply-To: <cover.1354677952.git.lv.zheng@intel.com>
Tranditional UARTs are used as communication pipes between the hosts
while the modern computing systems equipped with HSU (High Speed UARTs)
would connect on-board target devices using the UART ports. The role of
the UART controllers is changed from the communication facility to the
platform bus.
In the recent ACPI 5.0 specification updates, firmwares are provided the
possibilities to enumerate the UART target devices known to the platform
vendors.
Thus there are the needs for enumerating the UART target devices:
1. hotplug uevent
2. serial configuration
Currently, only serial cards on the specific bus (ex. PCMCIA) can be
enumerated and userspace can obtain the hotplug event of the UART target
devices. Linux kernel is lack of an overall enumeration mechanism for
UART target devices.
In order to send uevent, a device need to be a class device or a bus
device. This patch introduces a bus_type subsystem to manage the new
UART target device type for the purpose of being ready for the possible
future extensions.
When the UART target devices are created, userspace uevent rules can
pass the creation details to the userspace driver managers
(ex. hciattach). Here is an example of the uevents and exported
attributes of these new UART target devices:
Test DSDT (dummy UART host adapter INTF000 and target device INTF001):
Device (UA00)
{
Name (_HID, "INTF000") // _HID: Hardware ID
Name (RBUF, ResourceTemplate ()
{
Memory32Fixed (ReadWrite,
0x00000000, // Address Base
0x00001000) // Address Length
})
Method (_CRS, 0, NotSerialized) // _CRS: Current Resource Settings
{
Return (RBUF)
}
Method (_STA, 0, NotSerialized) // _STA: Status
{
Return (0x0F)
}
Device (BTH0)
{
Name (_HID, "INTF001") // _HID: Hardware ID
Method (_CRS, 0, NotSerialized) // _CRS: Current Resource Settings
{
Name (UBUF, ResourceTemplate ()
{
UartSerialBus (0x0001C200, DataBitsEight, StopBitsOne,
0xC0, LittleEndian, ParityTypeNone, FlowControlHardware,
0x0020, 0x0020, "\\_SB.PCI0.UA00",
0x00, ResourceConsumer, ,
)
})
Return (UBUF)
}
Method (_STA, 0, NotSerialized) // _STA: Status
{
Return (0x0F)
}
}
}
uevent and environments:
KERNEL[252.443458] add /bus/uart (bus)
ACTION=add
DEVPATH=/bus/uart
SEQNUM=1142
SUBSYSTEM=bus
KERNEL[268.491709] add /devices/platform/INTF000:00/INTF001:00 (uart)
ACTION=add
DEVPATH=/devices/platform/INTF000:00/INTF001:00
DEVTYPE=uart_device
MODALIAS=uart:INTF001:00
SEQNUM=1144
SUBSYSTEM=uart
kobject attribute files:
# cat /sys/bus/uart/devices/INTF001:00/modalias
uart:INTF001:00
# cat /sys/bus/uart/devices/INTF001:00/tty_dev
ttyS0
# cat /sys/bus/uart/devices/INTF001:00/tty_attrs
115200 8N0 HW
# cat /sys/bus/uart/devices/INTF001:00/modem_lines
LE:RTS,CTS,
kobject sysfs links:
# ls -l /sys/bus/uart/devices
INTF001:00 -> ../../../devices/platform/INTF000:00/INTF001:00
# ls -l /sys/devices/platform/INTF000:00/INTF001:00
subsystem -> ../../../../bus/uart
host_node -> ../tty/ttyS0
# ls -l /sys/devices/platform/INTF000:00/tty/ttyS0
target_node -> ../../INTF001:00
History:
v1:
1. Add uart bus type subsystem.
2. Add physical uart target device (uart_device).
3. Add logical uart host adapter (klist).
4. Add uart target device attributes.
5. Create tty_device<->uart_device links.
v2.
1. Change uart driver related stuff to tty driver.
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
drivers/tty/serial/Makefile | 2 +-
drivers/tty/serial/serial_bus.c | 412 +++++++++++++++++++++++++++++++++++++++
include/linux/mod_devicetable.h | 5 +
include/linux/serial_bus.h | 75 +++++++
4 files changed, 493 insertions(+), 1 deletion(-)
create mode 100644 drivers/tty/serial/serial_bus.c
create mode 100644 include/linux/serial_bus.h
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index 4f694da..4400521 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -2,7 +2,7 @@
# Makefile for the kernel serial device drivers.
#
-obj-$(CONFIG_SERIAL_CORE) += serial_core.o
+obj-$(CONFIG_SERIAL_CORE) += serial_core.o serial_bus.o
obj-$(CONFIG_SERIAL_21285) += 21285.o
# These Sparc drivers have to appear before others such as 8250
diff --git a/drivers/tty/serial/serial_bus.c b/drivers/tty/serial/serial_bus.c
new file mode 100644
index 0000000..afbb885
--- /dev/null
+++ b/drivers/tty/serial/serial_bus.c
@@ -0,0 +1,412 @@
+/*
+ * serial_bus.c - UART bus implementation
+ *
+ * Copyright (c) 2012, Intel Corporation
+ * Author: Lv Zheng <lv.zheng@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ */
+
+/*
+ * Tranditional UARTs are used as communication pipes between the hosts
+ * while the modern computing systems equipped with HSU (High Speed UARTs)
+ * would connect on-board target devices using the UART ports. The role of
+ * the UART controllers are changed from the communication facility to the
+ * platform bus.
+ *
+ * UART target devices are created in the kernel as struct uart_device.
+ * It is defined for the following purposes:
+ * 1. Sending hotplug notifications to the userspace
+ * 2. Exporting serial configuration parameters to the userspace
+ * 3. Allowing target device based PM to be added easily
+ *
+ */
+
+#include <linux/serial.h>
+#include <linux/serial_bus.h>
+#include <linux/slab.h>
+#include <linux/init.h>
+
+
+struct uart_match {
+ dev_t devt;
+};
+
+static int do_uart_tty_find(struct device *dev, void *data)
+{
+ struct uart_match *match = data;
+ return dev->devt == match->devt;
+}
+
+/**
+ * uart_tty_find - find the tty device using the line number
+ * @drv: the tty host driver
+ * @line: the line number of the tty device
+ * @dev: the physical uart host adapter
+ *
+ * Return a matching tty device on the host adapter.
+ */
+struct device *uart_tty_find(struct tty_driver *drv, unsigned int line,
+ struct device *dev)
+{
+ struct uart_match match;
+
+ match.devt = MKDEV(drv->major, drv->minor_start + line);
+
+ return device_find_child(dev, &match, do_uart_tty_find);
+}
+EXPORT_SYMBOL_GPL(uart_tty_find);
+
+/**
+ * uart_tty_name - get the name of the tty device according to the line
+ * number
+ * @drv: the tty host driver
+ * @line: the line number of the tty device
+ * @p: pointer to the buffer containing the returned name
+ *
+ * Return the name of the tty device.
+ */
+void uart_tty_name(struct tty_driver *drv, unsigned int line, char *p)
+{
+ BUG_ON(!drv || !p);
+
+ if (drv->flags & TTY_DRIVER_UNNUMBERED_NODE)
+ strcpy(p, drv->name);
+ else
+ sprintf(p, "%s%d", drv->name, line + drv->name_base);
+}
+EXPORT_SYMBOL_GPL(uart_tty_name);
+
+static ssize_t uart_dev_show_name(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct uart_device *udev = to_uart_device(dev);
+ return sprintf(buf, "%s\n", udev->name);
+}
+
+static ssize_t uart_dev_show_modalias(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct uart_device *udev = to_uart_device(dev);
+ return sprintf(buf, "%s%s\n", UART_MODULE_PREFIX, udev->name);
+}
+
+static ssize_t uart_dev_show_tty_dev(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct uart_device *udev = to_uart_device(dev);
+ return sprintf(buf, "%s\n", udev->tty_name);
+}
+
+static ssize_t uart_dev_show_tty_attrs(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct uart_device *udev = to_uart_device(dev);
+ int len = 0;
+
+ /* baud rate */
+ len += sprintf(buf+len, "%d ", udev->baud);
+
+ /* data bits */
+ switch (udev->cflag & CSIZE) {
+ case CS5:
+ len += sprintf(buf+len, "5");
+ break;
+ case CS6:
+ len += sprintf(buf+len, "6");
+ break;
+ case CS7:
+ len += sprintf(buf+len, "7");
+ break;
+ case CS8:
+ default:
+ len += sprintf(buf+len, "8");
+ break;
+ }
+
+ /* parity */
+ if (udev->cflag & PARODD)
+ len += sprintf(buf+len, "O");
+ else if (udev->cflag & PARENB)
+ len += sprintf(buf+len, "E");
+ else
+ len += sprintf(buf+len, "N");
+
+ /* stop bits */
+ len += sprintf(buf+len, "%d", udev->cflag & CSTOPB ? 1 : 0);
+
+ /* HW/SW control */
+ if (udev->cflag & CRTSCTS)
+ len += sprintf(buf+len, " HW");
+ if ((udev->iflag & (IXON|IXOFF|IXANY)) != 0)
+ len += sprintf(buf+len, " SW");
+
+ len += sprintf(buf+len, "\n");
+ return len;
+}
+
+static ssize_t uart_dev_show_modem_lines(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct uart_device *udev = to_uart_device(dev);
+ int len = 0;
+
+ /* endian */
+ if (udev->mctrl & TIOCM_LE)
+ len += sprintf(buf+len, "LE:");
+ else
+ len += sprintf(buf+len, "BE:");
+
+ /* terminal lines */
+ if (udev->mctrl & TIOCM_DTR)
+ len += sprintf(buf+len, "DTR,");
+ if (udev->mctrl & TIOCM_RTS)
+ len += sprintf(buf+len, "RTS,");
+
+ /* modem lines */
+ if (udev->mctrl & TIOCM_CTS)
+ len += sprintf(buf+len, "CTS,");
+ if (udev->mctrl & TIOCM_CAR)
+ len += sprintf(buf+len, "CAR,");
+ if (udev->mctrl & TIOCM_RNG)
+ len += sprintf(buf+len, "RNG,");
+ if (udev->mctrl & TIOCM_DSR)
+ len += sprintf(buf+len, "DSR,");
+
+ len += sprintf(buf+len, "\n");
+ return len;
+}
+
+static DEVICE_ATTR(name, S_IRUGO, uart_dev_show_name, NULL);
+static DEVICE_ATTR(modalias, S_IRUGO, uart_dev_show_modalias, NULL);
+static DEVICE_ATTR(tty_dev, S_IRUGO, uart_dev_show_tty_dev, NULL);
+static DEVICE_ATTR(tty_attrs, S_IRUGO, uart_dev_show_tty_attrs, NULL);
+static DEVICE_ATTR(modem_lines, S_IRUGO, uart_dev_show_modem_lines, NULL);
+
+static struct attribute *uart_dev_attrs[] = {
+ &dev_attr_name.attr,
+ /* coldplug: modprobe $(cat .../modalias) */
+ &dev_attr_modalias.attr,
+ &dev_attr_tty_dev.attr,
+ &dev_attr_tty_attrs.attr,
+ &dev_attr_modem_lines.attr,
+ NULL,
+};
+
+static struct attribute_group uart_dev_attr_group = {
+ .attrs = uart_dev_attrs,
+};
+
+static const struct attribute_group *uart_dev_attr_groups[] = {
+ &uart_dev_attr_group,
+ NULL,
+};
+
+#ifdef CONFIG_HOTPLUG
+static int uart_device_uevent(struct device *dev, struct kobj_uevent_env *env)
+{
+ struct uart_device *udev = to_uart_device(dev);
+
+ if (add_uevent_var(env, "MODALIAS=%s%s",
+ UART_MODULE_PREFIX, udev->name))
+ return -ENOMEM;
+
+ dev_dbg(dev, "uevent\n");
+ return 0;
+}
+#else
+#define uart_device_uevent NULL
+#endif
+
+static void uart_device_release(struct device *dev)
+{
+ struct uart_device *udev = to_uart_device(dev);
+
+ put_device(udev->tty);
+ kfree(udev);
+}
+
+struct device_type uart_device_type = {
+ .name = "uart_device",
+ .groups = uart_dev_attr_groups,
+ .uevent = uart_device_uevent,
+ .release = uart_device_release,
+};
+EXPORT_SYMBOL_GPL(uart_device_type);
+
+/**
+ * uart_register_device - register a physical uart target device
+ * @adap: the logical uart host adapter
+ * @dev: the physical uart host adapter
+ * @drv: the tty host driver
+ * @info: the uart target device description
+ *
+ * Initialize and add a physical uart target device.
+ *
+ * This returns the new uart target device, which may be saved for later use
+ * with uart_unregister_device; or NULL to indicate an error.
+ */
+struct uart_device *uart_register_device(struct klist *adap,
+ struct device *dev,
+ struct tty_driver *drv,
+ struct uart_board_info const *info)
+{
+ struct uart_device *udev;
+ struct device *tty;
+ int status;
+
+ BUG_ON((!adap && !dev) || !drv);
+
+ udev = kzalloc(sizeof(struct uart_device), GFP_KERNEL);
+ if (!udev)
+ return NULL;
+
+ strlcpy(udev->name, info->type, sizeof(udev->name));
+
+ udev->baud = info->baud;
+ udev->cflag = info->cflag;
+ udev->iflag = info->iflag;
+ udev->mctrl = info->mctrl;
+
+ udev->dev.parent = dev;
+ udev->dev.bus = &uart_bus_type;
+ udev->dev.type = &uart_device_type;
+
+ tty = uart_tty_find(drv, info->line, dev);
+ if (!tty) {
+ dev_err(dev, "Cannot find associated tty device at line %d.\n",
+ info->line);
+ goto fail;
+ }
+ udev->tty = get_device(tty);
+ uart_tty_name(drv, info->line, udev->tty_name);
+
+ dev_set_name(&udev->dev, "%s", udev->name);
+ status = device_register(&udev->dev);
+ if (status) {
+ dev_err(dev,
+ "Failed to register uart target device at line %d.\n",
+ info->line);
+ goto fail2;
+ }
+ dev_info(&udev->dev, "Registered at line %d.\n", info->line);
+
+ status = sysfs_create_link(&tty->kobj, &udev->dev.kobj, "target_node");
+ status = sysfs_create_link(&udev->dev.kobj, &tty->kobj, "host_node");
+
+ if (adap) {
+ udev->adap = adap;
+ klist_add_tail(&udev->klist_parent, adap);
+ }
+
+ return udev;
+
+fail2:
+ put_device(tty);
+fail:
+ kfree(udev);
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(uart_register_device);
+
+/**
+ * uart_unregister_device - unregister a physical uart target device
+ * @udev: the physical uart target device
+ *
+ * Reverse effect of uart_register_device().
+ */
+void uart_unregister_device(struct uart_device *udev)
+{
+ if (!udev)
+ return;
+ dev_info(&udev->dev, "Unregistering...\n");
+
+ if (udev->adap)
+ klist_del(&udev->klist_parent);
+ sysfs_remove_link(&udev->dev.kobj, "host_node");
+ if (udev->tty)
+ sysfs_remove_link(&udev->tty->kobj, "target_node");
+ device_unregister(&udev->dev);
+}
+EXPORT_SYMBOL_GPL(uart_unregister_device);
+
+static void klist_uart_get(struct klist_node *n)
+{
+ struct uart_device *udev = uart_device_from_parent(n);
+ get_device(&udev->dev);
+}
+
+static void klist_uart_put(struct klist_node *n)
+{
+ struct uart_device *udev = uart_device_from_parent(n);
+ put_device(&udev->dev);
+}
+
+static struct uart_device *uart_next_device(struct klist_iter *i)
+{
+ struct klist_node *n = klist_next(i);
+ return n ? uart_device_from_parent(n) : NULL;
+}
+
+/**
+ * uart_add_adapter - register a logical uart host adapter
+ * @adap: the logical uart host adapter
+ *
+ * Initialize a logical uart host adapter.
+ */
+int uart_add_adapter(struct klist *adap)
+{
+ klist_init(adap, klist_uart_get, klist_uart_put);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(uart_add_adapter);
+
+/**
+ * uart_del_adapter - unregister a logical uart host adapter
+ * @adap: the logical uart host adapter
+ *
+ * Reverse effect of uart_add_adapter().
+ */
+void uart_del_adapter(struct klist *adap)
+{
+ struct klist_iter i;
+ struct uart_device *udev;
+
+ if (!adap)
+ return;
+
+ klist_iter_init(adap, &i);
+ while ((udev = uart_next_device(&i)))
+ uart_unregister_device(udev);
+ klist_iter_exit(&i);
+}
+EXPORT_SYMBOL_GPL(uart_del_adapter);
+
+struct bus_type uart_bus_type = {
+ .name = "uart",
+};
+EXPORT_SYMBOL_GPL(uart_bus_type);
+
+static int __init uart_bus_init(void)
+{
+ int retval;
+
+ retval = bus_register(&uart_bus_type);
+ if (retval)
+ return retval;
+
+ return 0;
+}
+
+static void __exit uart_bus_exit(void)
+{
+ bus_unregister(&uart_bus_type);
+}
+
+subsys_initcall(uart_bus_init);
+module_exit(uart_bus_exit);
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index fed3def..28df140 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -455,6 +455,11 @@ struct spi_device_id {
__attribute__((aligned(sizeof(kernel_ulong_t))));
};
+/* uart */
+
+#define UART_NAME_SIZE 32
+#define UART_MODULE_PREFIX "uart:"
+
/* dmi */
enum dmi_field {
DMI_NONE,
diff --git a/include/linux/serial_bus.h b/include/linux/serial_bus.h
new file mode 100644
index 0000000..6d66fe2
--- /dev/null
+++ b/include/linux/serial_bus.h
@@ -0,0 +1,75 @@
+/*
+ * serial_bus.h - UART bus implementation
+ *
+ * Copyright (c) 2012, Intel Corporation
+ * Author: Lv Zheng <lv.zheng@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ */
+#ifndef LINUX_SERIAL_BUS_H
+#define LINUX_SERIAL_BUS_H
+
+#include <linux/compiler.h>
+#include <linux/tty.h>
+#include <linux/device.h>
+#include <linux/mod_devicetable.h>
+
+struct uart_device;
+
+/*
+ * UART Bus
+ */
+struct uart_board_info {
+ char type[UART_NAME_SIZE];
+ unsigned int line; /* port index */
+ unsigned int cflag; /* termio cflag */
+ unsigned int iflag; /* termio iflag */
+ unsigned int mctrl; /* modem ctrl settings */
+ unsigned int baud;
+ int irq;
+};
+
+extern struct bus_type uart_bus_type;
+
+int uart_add_adapter(struct klist *adap);
+void uart_del_adapter(struct klist *adap);
+
+struct uart_device {
+ char name[UART_NAME_SIZE];
+ char tty_name[64];
+ unsigned int cflag; /* termio cflag */
+ unsigned int iflag; /* termio iflag */
+ unsigned int mctrl; /* modem ctrl settings */
+ unsigned int baud;
+ struct device dev;
+ struct device *tty;
+ struct klist_node klist_parent;
+ struct klist *adap; /* set for multi-port adapter */
+};
+
+extern struct device_type uart_device_type;
+
+#define is_uart_device(d) ((d) && (d)->type == &uart_device_type)
+#define to_uart_device(d) container_of(d, struct uart_device, dev)
+#define uart_device_from_parent(n) \
+ container_of(n, struct uart_device, klist_parent)
+
+static inline struct uart_device *uart_verify_device(struct device *dev)
+{
+ return is_uart_device(dev) ? to_uart_device(dev) : NULL;
+}
+
+struct uart_device *uart_register_device(struct klist *adap,
+ struct device *dev,
+ struct tty_driver *drv,
+ struct uart_board_info const *info);
+void uart_unregister_device(struct uart_device *udev);
+
+struct device *uart_tty_find(struct tty_driver *drv, unsigned int line,
+ struct device *dev);
+void uart_tty_name(struct tty_driver *driver, unsigned int line, char *p);
+
+#endif /* LINUX_SERIAL_BUS_H */
--
1.7.10
^ permalink raw reply related
* [PATCH v2 0/4] ACPI/UART: Add ACPI 5.0 enueration support for UART.
From: Lv Zheng @ 2012-12-05 3:51 UTC (permalink / raw)
To: Len Brown, Rafael J Wysocki, Greg Kroah-Hartman, Alan Cox,
Mika Westerberg
Cc: linux-acpi, linux-serial, Lv Zheng
In-Reply-To: <cover.1354505471.git.lv.zheng@intel.com>
ACPI 5.0 specification introduces enumeration support for SPB buses. This
patch set adds the UART serial bus enumeration support to Linux using such
mechanism.
NOTE: The [PATCH 4/4] is only for the demonstration purpose and should not
be merged into any of the published Linux source tree.
Lv Zheng (4):
UART: Add UART subsystem as a bus.
ACPI / UART: Add ACPI enumeration support for UART bus.
UART / 8250: Add declearation of serial8250 driver.
UART: Add dummy devices to test the enumeration.
drivers/acpi/Kconfig | 7 +
drivers/acpi/Makefile | 1 +
drivers/acpi/acpi_uart.c | 262 +++++++++++++++++++++
drivers/acpi/scan.c | 1 +
drivers/tty/serial/8250/8250.c | 17 +-
drivers/tty/serial/8250/8250_dummy.c | 129 ++++++++++
drivers/tty/serial/8250/Kconfig | 10 +
drivers/tty/serial/8250/Makefile | 1 +
drivers/tty/serial/Makefile | 2 +-
drivers/tty/serial/serial_bus.c | 429 ++++++++++++++++++++++++++++++++++
include/linux/acpi_uart.h | 40 ++++
include/linux/mod_devicetable.h | 5 +
include/linux/serial_8250.h | 2 +
include/linux/serial_bus.h | 80 +++++++
14 files changed, 982 insertions(+), 4 deletions(-)
create mode 100644 drivers/acpi/acpi_uart.c
create mode 100644 drivers/tty/serial/8250/8250_dummy.c
create mode 100644 drivers/tty/serial/serial_bus.c
create mode 100644 include/linux/acpi_uart.h
create mode 100644 include/linux/serial_bus.h
--
1.7.10
^ permalink raw reply
* RE: [RFC PATCH 1/3] UART: Add UART subsystem as a bus.
From: Zheng, Lv @ 2012-12-05 3:49 UTC (permalink / raw)
To: Mika Westerberg
Cc: Brown, Len, Wysocki, Rafael J, Greg Kroah-Hartman, Alan Cox,
linux-acpi@vger.kernel.org, linux-serial@vger.kernel.org
In-Reply-To: <20121204185404.GQ3117@intel.com>
> On Mon, Dec 03, 2012 at 11:39:59AM +0800, Lv Zheng wrote:
> > kobject attribute files:
> > # cat /sys/bus/uart/devices/INTF001:00/modalias
> > uart:INTF001:00
> > # cat /sys/bus/uart/devices/INTF001:00/tty_dev
> > ttyS0
> > # cat /sys/bus/uart/devices/INTF001:00/tty_attrs
> > 115200 8N0 HW
> > # cat /sys/bus/uart/devices/INTF001:00/modem_lines
> > LE:RTS,CTS,
>
> What if instead of exporting these to userspace we just set the defaults based
> on the UartSerialBus() value? This way the user only needs to find the right tty
> to pass to hciattach.
>
> > kobject sysfs links:
> > # ls -l /sys/bus/uart/devices
> > INTF001:00 -> ../../../devices/platform/INTF000:00/INTF001:00
> > # ls -l /sys/devices/platform/INTF000:00/INTF001:00
> > subsystem -> ../../../../bus/uart
> > host_node -> ../tty/ttyS0
> > # ls -l /sys/devices/platform/INTF000:00/tty/ttyS0
> > target_node -> ../../INTF001:00
>
> And if we have enumerated the UART controller from ACPI (it is probably
> attached to the platform bus) we can find the tty device it exports like:
>
> /sys/bus/platform/devices/INTF000:00/tty/ttyS1/dev
>
> Only thing that is missing then is the type of the connected device. That can be
> extracted by following the firmware_node symlink:
>
> # ls /sys/bus/platform/devices/INTF000\:00/firmware_node/
> INTF001:00
> ...
>
> # cat
> /sys/bus/platform/devices/INTF000\:00/firmware_node/INTF001:00/hid
> INTF001
>
> This ACPI ID then can be used to find out what device it is.
>
> Well, this is probably not the simplest solution :-) I'm just trying to understand
> why we need to add a new bus type if only thing it adds is the sysfs files.
In our team, we've been discussing this solution like your suggestion for some times.
ACPI UART enumeration can be done in this way without kernel modifications.
IMO:
1. tty_device is current used as "host" side device, you can find .pm support in most of the serial drivers.
They are always the codes turning off/on the clock of the host controller.
If we want to support PM for a target device, there is no such node in the kernel device hierarchy.
2. firmware_node is based on the "likely deprecated" acpi_device solution, I need to find another one. Maybe this is still a shadow device.
Like dmi-id device, a shadow device exporting attributes and generating uevents for the user space drivers may be helpful.
It would be better if the PM support of the target devices can benefit from the existence of the shadow uart target devices.
> > +/**
> > + * uart_unregister_device - unregister a UART device
> > + * @device: value returned from uart_register_device()
> > + *
> > + * Reverse effect of uart_register_device().
> > + */
> > +void uart_unregister_device(struct uart_device *udev) {
> > + if (udev->adap)
> > + klist_del(&udev->klist_parent);
> > + sysfs_remove_link(&udev->dev.kobj, "host_node");
> > + if (udev->tty)
> > + sysfs_remove_link(&udev->tty->kobj, "target_node");
> > + device_unregister(&udev->dev);
> > +}
> > +EXPORT_SYMBOL_GPL(uart_unreigster_device);
>
> Typo in the function name.
Fixed...
Thanks and best regards/Lv Zheng
^ permalink raw reply
* RE: [RFC PATCH 1/3] UART: Add UART subsystem as a bus.
From: Zheng, Lv @ 2012-12-05 3:37 UTC (permalink / raw)
To: Alan Cox
Cc: Brown, Len, Wysocki, Rafael J, Greg Kroah-Hartman,
linux-acpi@vger.kernel.org, linux-serial@vger.kernel.org
In-Reply-To: <20121203114604.079a2721@bob.linux.org.uk>
OK, all uart_driver and serial_core.h stuff will be removed from the next revision.
Best regards/Lv Zheng
> -----Original Message-----
> From: linux-serial-owner@vger.kernel.org
> [mailto:linux-serial-owner@vger.kernel.org] On Behalf Of Alan Cox
> Sent: Monday, December 03, 2012 7:46 PM
> To: Zheng, Lv
> Cc: Brown, Len; Wysocki, Rafael J; Greg Kroah-Hartman;
> linux-acpi@vger.kernel.org; linux-serial@vger.kernel.org
> Subject: Re: [RFC PATCH 1/3] UART: Add UART subsystem as a bus.
>
> > + * uart_tty_find - find the associated TTY device for the UART target
> > + * device
> > + * @drv: the low level UART driver
> > + * @dev: the parent physical device for the TTY devices
> > + * @line: the line number of the UART target device
> > + *
>
> Our top level abstraction is struct tty and struct tty_port. There is nothing
> requiring a serial tty is using the uart helper layer, nor should there be, so your
> code needs to support devices not using the uart layer. Otherwise it looks quite
> reasonable.
>
> our 3.7 tty layer has sysfs nodes on the tty which may also help
>
> Alan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body
> of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] synclink fix ldisc buffer argument
From: Chen Gang @ 2012-12-05 1:57 UTC (permalink / raw)
To: Paul Fulghum; +Cc: Alan Cox, Greg KH, Linux Kernel Mailing List, linux-serial
In-Reply-To: <50BCCD4C.8050209@microgate.com>
于 2012年12月04日 00:03, Paul Fulghum 写道:
> On 12/2/2012 8:20 PM, Chen Gang wrote:
>> pardon (I am just learning)
>> does 65535 mean HDLC_MAX_FRAME_SIZE ?
>> why do we need info->max_frame_size >= 4096 ?
>> in drivers/tty/synclink_gt.c:
>> 3550 if (info->max_frame_size < 4096)
>> 3551 info->max_frame_size = 4096;
>> 3552 else if (info->max_frame_size > 65535)
>> 3553 info->max_frame_size = 65535;
>> 3554
>> ...
>> 3603 info->max_frame_size = 4096;
>
> The hardware can send and receive HDLC frames up to
> 64K in size. The driver defaults to 4K max frame size
> to save buffer space for the common case
> (line 3603 in alloc_dev()).
>
> The module parameter max_frame_size can override the default
> in add_device() (lines 3550-3554 are from add_device()
> range checking the module parameter)
>
thank you.
sorry for reply late (yesterday, I have an annual leave for personal things, and not connect net).
by the way:
does it also need check the length in function rx_get_buf ?
(it seems not, but I am not quite sure, can you give a confirm ?)
4779 /*
4780 * pass receive buffer (RAW synchronous mode) to tty layer
4781 * return true if buffer available, otherwise false
4782 */
4783 static bool rx_get_buf(struct slgt_info *info)
4784 {
4785 unsigned int i = info->rbuf_current;
4786 unsigned int count;
4787
4788 if (!desc_complete(info->rbufs[i]))
4789 return false;
4790 count = desc_count(info->rbufs[i]);
4791 switch(info->params.mode) {
4792 case MGSL_MODE_MONOSYNC:
4793 case MGSL_MODE_BISYNC:
4794 case MGSL_MODE_XSYNC:
4795 /* ignore residue in byte synchronous modes */
4796 if (desc_residue(info->rbufs[i]))
4797 count--;
4798 break;
4799 }
4800 DBGDATA(info, info->rbufs[i].buf, count, "rx");
4801 DBGINFO(("rx_get_buf size=%d\n", count));
4802 if (count)
4803 ldisc_receive_buf(info->port.tty, info->rbufs[i].buf,
4804 info->flag_buf, count);
4805 free_rbufs(info, i, i);
4806 return true;
4807 }
--
Chen Gang
Asianux Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] synclink fix ldisc buffer argument
From: Chen Gang @ 2012-12-05 1:35 UTC (permalink / raw)
To: Paul Fulghum; +Cc: Greg KH, Linux Kernel Mailing List, linux-serial, Alan Cox
In-Reply-To: <50BCDDB4.6010608@microgate.com>
于 2012年12月04日 01:13, Paul Fulghum 写道:
> Fix call to line discipline receive_buf by synclink drivers.
> Dummy flag buffer argument is ignored by N_HDLC line discipline but might
> be of insufficient size if accessed by a different line discipline
> selected by mistake. flag buffer allocation now matches max size of data
> buffer. Unused char_buf buffers are removed.
>
> Signed-off-by: Paul Fulghum <paulkf@microgate.com>
>
thank you very much.
might we use macro instead of hard code number ? (such as 4096,
65535). it is only an idea, not means need regression.
thanks.
--
Chen Gang
Asianux Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC PATCH 1/3] UART: Add UART subsystem as a bus.
From: Alan Cox @ 2012-12-04 19:50 UTC (permalink / raw)
To: Mika Westerberg
Cc: Lv Zheng, Len Brown, Rafael J Wysocki, Greg Kroah-Hartman,
linux-acpi, linux-serial
In-Reply-To: <20121204185404.GQ3117@intel.com>
> What if instead of exporting these to userspace we just set the
> defaults based on the UartSerialBus() value? This way the user only
> needs to find the right tty to pass to hciattach.
We should also make sure the types are generic because there are lots
of platforms that can use DT to describe devices usefully this way, and
there are types not in the ACPI spec (eg Loconet) that some apps could
do with finding.
> And if we have enumerated the UART controller from ACPI (it is
> probably attached to the platform bus) we can find the tty device it
> exports like:
The property should not be in any ACPI specific form or space - just
attach it directly to the tty from ACPI, DT, driver internal knowledge,
PCI id, whatever
Alan
^ permalink raw reply
* Re: flush_to_ldisc accesses tty after free (was: [PATCH 21/21] TTY: move tty buffers to tty_port)
From: Ilya Zykov @ 2012-12-04 19:21 UTC (permalink / raw)
To: Peter Hurley
Cc: Sasha Levin, Jiri Slaby, Jiri Slaby, gregkh, alan, linux-kernel,
Dave Jones, linux-serial
In-Reply-To: <1354373995.2531.48.camel@thor>
On 01.12.2012 18:59, Peter Hurley wrote:
> (cc'ing Ilya Zykov <ilya@ilyx.ru> because the test jig below is based on
> his test program from https://lkml.org/lkml/2012/11/29/368 -- just want
> to give credit where credit is due)
>
> On Fri, 2012-11-30 at 18:52 -0500, Sasha Levin wrote:
>>
>> Still reproducible, I'm still seeing this with the patch above applied:
>>
>> [ 1315.419759] ------------[ cut here ]------------
>> [ 1315.420611] WARNING: at drivers/tty/tty_buffer.c:476 flush_to_ldisc+0x60/0x200()
>> [ 1315.423098] tty is NULL
>
> Thanks for sticking with this Sasha. Finally me too.
>
> ---
> [ 88.331234] WARNING: at /home/peter/src/kernels/next/drivers/tty/tty_buffer.c:435 flush_to_ldisc+0x194/0x1d0()
> [ 88.334505] Hardware name: Bochs
> [ 88.335618] tty is bad=-1
> [ 88.335703] Modules linked in: netconsole configfs bnep rfcomm bluetooth snd_hda_intel snd_hda_codec snd_hwdep
> parport_pc ppdev snd_pcm snd_seq_midi snd_rawmidi snd_seq_midi_event snd_seq snd_timer snd_seq_device mac_hid
> psmouse snd i2c_piix4 soundcore snd_page_alloc microcode serio_raw virtio_balloon lp parport floppy 8139too 8139cp
> [ 88.345272] Pid: 39, comm: kworker/1:1 Tainted: G W 3.7.0-next-20121129+ttydebug-xeon #20121129+ttydebug
> [ 88.347736] Call Trace:
> [ 88.349024] [<ffffffff81058aff>] warn_slowpath_common+0x7f/0xc0
> [ 88.350383] [<ffffffff81058bf6>] warn_slowpath_fmt+0x46/0x50
> [ 88.351745] [<ffffffff81432bd4>] flush_to_ldisc+0x194/0x1d0
> [ 88.353047] [<ffffffff816f7fe1>] ? _raw_spin_unlock_irq+0x21/0x50
> [ 88.354190] [<ffffffff8108a809>] ? finish_task_switch+0x49/0xe0
> [ 88.355436] [<ffffffff81077ad1>] process_one_work+0x121/0x490
> [ 88.357674] [<ffffffff81432a40>] ? __tty_buffer_flush+0x90/0x90
> [ 88.358954] [<ffffffff81078c84>] worker_thread+0x164/0x3e0
> [ 88.360247] [<ffffffff81078b20>] ? manage_workers+0x120/0x120
> [ 88.361282] [<ffffffff8107e230>] kthread+0xc0/0xd0
> [ 88.362284] [<ffffffff816f0000>] ? cmos_do_probe+0x2eb/0x3bf
> [ 88.363391] [<ffffffff8107e170>] ? flush_kthread_worker+0xb0/0xb0
> [ 88.364797] [<ffffffff816fff6c>] ret_from_fork+0x7c/0xb0
> [ 88.366087] [<ffffffff8107e170>] ? flush_kthread_worker+0xb0/0xb0
> [ 88.367266] ---[ end trace 453a7c9f38fbfec0 ]---
>
>
> I figured out how to make this reproduce easily. The test jig at the end
> of this email will generate this multiple times a second.
>
> The test creates a pty pair and spawns a child which writes to the slave
> pts, while the parent waits for the first write and then abruptly closes
> the master ptm and kills the child. (Just in case, I'd only run the jig
> in a disposable vm. Obviously, the vm needs multiple cores and extra pty
> serial devices ;)
>
>>From instrumenting the tty_release() path, it's clear that tty_buffer
> work is still scheduled even after tty_release_ldisc() has run. For
> example, with this patch I get the warning below it.
>
> [Further analysis to follow in subsequent mail...]
>
> --- >% ---
> [PATCH -next] tty: WARN if buffer work racing with tty free
>
>
> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> ---
> drivers/tty/tty_io.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index 1ce50ec..9d53aec 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -1511,6 +1511,8 @@ static void queue_release_one_tty(struct kref *kref)
> {
> struct tty_struct *tty = container_of(kref, struct tty_struct, kref);
>
> + WARN_ON(work_pending(&tty->port->buf.work));
> +
> /* The hangup queue is now free so we can reuse it rather than
> waste a chunk of memory for each port */
> INIT_WORK(&tty->hangup_work, release_one_tty);
>
> /*
> * pty_thrash.c
> *
> * Based on original test jig by Ilya Zykov <ilya@ilyx.ru>
> */
Yes, ok with me.
Signed-off-by: Ilya Zykov <ilya@ilyx.ru>
>
> #include <stdio.h>
> #include <fcntl.h>
> #include <sys/ioctl.h>
> #include <termios.h>
> #include <stdlib.h>
> #include <errno.h>
> #include <string.h>
> #include <stdarg.h>
> #include <signal.h>
>
> #define parent child_id
>
> static int fd;
>
> static void error_exit(char *f, ...)
> {
> va_list va;
>
> va_start(va, f);
> vprintf(f, va);
> printf(": %s\n", strerror(errno));
> va_end(va);
>
> if (fd >= 0)
> close(fd);
>
> exit(EXIT_FAILURE);
> }
>
> int main(int argc, char *argv[]) {
> int parent;
> char pts_name[24];
> int ptn, unlock;
>
> while (1) {
>
> fd = open("/dev/ptmx", O_RDWR);
> if (fd < 0)
> error_exit("opening pty master");
> unlock = 0;
> if (ioctl(fd, TIOCSPTLCK, &unlock) < 0)
> error_exit("unlocking pty pair");
> if (ioctl(fd, TIOCGPTN, &ptn) < 0)
> error_exit("getting pty #");
> snprintf(pts_name, sizeof(pts_name), "/dev/pts/%d", ptn);
>
> child_id = fork();
> if (child_id == -1)
> error_exit("forking child");
>
> if (parent) {
> int err, id, status;
> char buf[128];
> int n;
>
> n = read(fd, buf, sizeof(buf));
> if (n < 0)
> error_exit("master reading");
> printf("%.*s\n", n-1, buf);
>
> close(fd);
>
> err = kill(child_id, SIGKILL);
> if (err < 0)
> error_exit("killing child");
> id = waitpid(child_id, &status, 0);
> if (id < 0 || id != child_id)
> error_exit("waiting for child");
>
> } else { /* Child */
>
> close(fd);
> printf("Test cycle on slave pty %s\n", pts_name);
> fd = open(pts_name, O_RDWR);
> if (fd < 0)
> error_exit("opening pty slave");
>
> while (1) {
> char pattern[] = "test\n";
> if (write(fd, pattern, strlen(pattern)) < 0)
> error_exit("slave writing");
> }
>
> }
> }
>
> /* never gets here */
> return 0;
> }
>
Always Welcome.
Ilya.
^ permalink raw reply
* Re: [RFC PATCH 1/3] UART: Add UART subsystem as a bus.
From: Mika Westerberg @ 2012-12-04 18:54 UTC (permalink / raw)
To: Lv Zheng
Cc: Len Brown, Rafael J Wysocki, Greg Kroah-Hartman, Alan Cox,
linux-acpi, linux-serial
In-Reply-To: <6d5fc2e0799c12554aa8acdb2d7782ba8643902b.1354505472.git.lv.zheng@intel.com>
On Mon, Dec 03, 2012 at 11:39:59AM +0800, Lv Zheng wrote:
> kobject attribute files:
> # cat /sys/bus/uart/devices/INTF001:00/modalias
> uart:INTF001:00
> # cat /sys/bus/uart/devices/INTF001:00/tty_dev
> ttyS0
> # cat /sys/bus/uart/devices/INTF001:00/tty_attrs
> 115200 8N0 HW
> # cat /sys/bus/uart/devices/INTF001:00/modem_lines
> LE:RTS,CTS,
What if instead of exporting these to userspace we just set the defaults
based on the UartSerialBus() value? This way the user only needs to find the
right tty to pass to hciattach.
> kobject sysfs links:
> # ls -l /sys/bus/uart/devices
> INTF001:00 -> ../../../devices/platform/INTF000:00/INTF001:00
> # ls -l /sys/devices/platform/INTF000:00/INTF001:00
> subsystem -> ../../../../bus/uart
> host_node -> ../tty/ttyS0
> # ls -l /sys/devices/platform/INTF000:00/tty/ttyS0
> target_node -> ../../INTF001:00
And if we have enumerated the UART controller from ACPI (it is probably
attached to the platform bus) we can find the tty device it exports like:
/sys/bus/platform/devices/INTF000:00/tty/ttyS1/dev
Only thing that is missing then is the type of the connected device. That
can be extracted by following the firmware_node symlink:
# ls /sys/bus/platform/devices/INTF000\:00/firmware_node/
INTF001:00
...
# cat /sys/bus/platform/devices/INTF000\:00/firmware_node/INTF001:00/hid
INTF001
This ACPI ID then can be used to find out what device it is.
Well, this is probably not the simplest solution :-) I'm just trying to
understand why we need to add a new bus type if only thing it adds is the
sysfs files.
> +/**
> + * uart_unregister_device - unregister a UART device
> + * @device: value returned from uart_register_device()
> + *
> + * Reverse effect of uart_register_device().
> + */
> +void uart_unregister_device(struct uart_device *udev)
> +{
> + if (udev->adap)
> + klist_del(&udev->klist_parent);
> + sysfs_remove_link(&udev->dev.kobj, "host_node");
> + if (udev->tty)
> + sysfs_remove_link(&udev->tty->kobj, "target_node");
> + device_unregister(&udev->dev);
> +}
> +EXPORT_SYMBOL_GPL(uart_unreigster_device);
Typo in the function name.
^ permalink raw reply
* Re: [PATCHv3 3/5] serial: 8250_dw: Map IO memory
From: Jamie Iles @ 2012-12-04 17:02 UTC (permalink / raw)
To: Heikki Krogerus
Cc: Greg Kroah-Hartman, Alan Cox, Jamie Iles, linux-serial, LKML
In-Reply-To: <1354634512-19299-1-git-send-email-heikki.krogerus@linux.intel.com>
Looks good to me!
On Tue, Dec 04, 2012 at 05:21:52PM +0200, Heikki Krogerus wrote:
> This needs to be done in order to later access the
> Designware specific registers.
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Reviewed-by: Jamie Iles <jamie@jamieiles.com>
^ permalink raw reply
* [PATCHv3 3/5] serial: 8250_dw: Map IO memory
From: Heikki Krogerus @ 2012-12-04 15:21 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Alan Cox, Jamie Iles, linux-serial, LKML
In-Reply-To: <20121204081713.GA8071@xps8300>
This needs to be done in order to later access the
Designware specific registers.
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
drivers/tty/serial/8250/8250_dw.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index ff83ea5..e174901 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -111,10 +111,13 @@ static int dw8250_probe(struct platform_device *pdev)
uart.port.irq = irq->start;
uart.port.handle_irq = dw8250_handle_irq;
uart.port.type = PORT_8250;
- uart.port.flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF | UPF_IOREMAP |
- UPF_FIXED_PORT;
+ uart.port.flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF | UPF_FIXED_PORT;
uart.port.dev = &pdev->dev;
+ uart.port.membase = ioremap(regs->start, resource_size(regs));
+ if (!uart.port.membase)
+ return -ENOMEM;
+
uart.port.iotype = UPIO_MEM;
uart.port.serial_in = dw8250_serial_in;
uart.port.serial_out = dw8250_serial_out;
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH -next 0/9] tty: Fix buffer work access-after-free
From: Alan Cox @ 2012-12-04 14:30 UTC (permalink / raw)
To: Peter Hurley
Cc: Alan Cox, Ilya Zykov, Jiri Slaby, Greg Kroah-Hartman,
linux-serial, linux-kernel
In-Reply-To: <1354629480.475.7.camel@thor>
> I assume you mean somewhere other than ml archives:
> https://lkml.org/lkml/2012/12/1/57
Putting in the commit message would be idea given how short it is.
>
> Setting up a mini-testsuite is good idea. As I noted in that email
> though, that test jig is a derivative work that I'd want a ok from Ilya
> Zykov to put somewhere more permanent.
Sure
^ permalink raw reply
* Re: [PATCH] tty: Correct tty buffer flush.
From: Alan Cox @ 2012-12-04 14:12 UTC (permalink / raw)
To: Ilya Zykov; +Cc: Greg Kroah-Hartman, Alan Cox, linux-kernel, linux-serial
In-Reply-To: <50BDF661.30303@ilyx.ru>
On Tue, 04 Dec 2012 17:10:57 +0400
Ilya Zykov <ilya@ilyx.ru> wrote:
> The root of problem is carelessly zeroing pointer(in function __tty_buffer_flush()),
> when another thread can use it. It can be cause of "NULL pointer dereference".
> Main idea of the patch, this is never free last (struct tty_buffer) in the active buffer.
> Only flush the data for ldisc(tty->buf.head->read = tty->buf.head->commit).
> At that moment driver can collect(write) data in buffer without conflict.
> It is repeat behavior of flush_to_ldisc(), only without feeding data to ldisc.
>
> Also revert:
> commit c56a00a165712fd73081f40044b1e64407bb1875
> tty: hold lock across tty buffer finding and buffer filling
> In order to delete the unneeded locks any more.
>
> Signed-off-by: Ilya Zykov <ilya@ilyx.ru>
Acked-by: Alan Cox <alan@linux.intel.com>
^ permalink raw reply
* Re: [PATCH -next 0/9] tty: Fix buffer work access-after-free
From: Peter Hurley @ 2012-12-04 13:58 UTC (permalink / raw)
To: Alan Cox, Ilya Zykov
Cc: Jiri Slaby, Greg Kroah-Hartman, linux-serial, linux-kernel
In-Reply-To: <20121204085426.389ca79b@bob.linux.org.uk>
On Tue, 2012-12-04 at 08:54 +0000, Alan Cox wrote:
> On Tue, 4 Dec 2012 02:07:36 -0500
> Peter Hurley <peter@hurleysoftware.com> wrote:
>
> > This patch series addresses the causes of flush_to_ldisc accessing
> > the tty after freeing.
>
> Looks good to me. Would be nice to keep a copy of the test that shows
> it up in the comments of the patches somewhere.
I assume you mean somewhere other than ml archives:
https://lkml.org/lkml/2012/12/1/57
Setting up a mini-testsuite is good idea. As I noted in that email
though, that test jig is a derivative work that I'd want a ok from Ilya
Zykov to put somewhere more permanent.
Regards,
Peter Hurley
^ permalink raw reply
* [PATCH] tty: Correct tty buffer flush.
From: Ilya Zykov @ 2012-12-04 13:10 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Alan Cox, linux-kernel, linux-serial
The root of problem is carelessly zeroing pointer(in function __tty_buffer_flush()),
when another thread can use it. It can be cause of "NULL pointer dereference".
Main idea of the patch, this is never free last (struct tty_buffer) in the active buffer.
Only flush the data for ldisc(tty->buf.head->read = tty->buf.head->commit).
At that moment driver can collect(write) data in buffer without conflict.
It is repeat behavior of flush_to_ldisc(), only without feeding data to ldisc.
Also revert:
commit c56a00a165712fd73081f40044b1e64407bb1875
tty: hold lock across tty buffer finding and buffer filling
In order to delete the unneeded locks any more.
Signed-off-by: Ilya Zykov <ilya@ilyx.ru>
---
drivers/tty/tty_buffer.c | 96 +++++++++++++---------------------------------
1 files changed, 27 insertions(+), 69 deletions(-)
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 91e326f..4f02f9c 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -114,11 +114,14 @@ static void __tty_buffer_flush(struct tty_struct *tty)
{
struct tty_buffer *thead;
- while ((thead = tty->buf.head) != NULL) {
- tty->buf.head = thead->next;
- tty_buffer_free(tty, thead);
+ if (tty->buf.head == NULL)
+ return;
+ while ((thead = tty->buf.head->next) != NULL) {
+ tty_buffer_free(tty, tty->buf.head);
+ tty->buf.head = thead;
}
- tty->buf.tail = NULL;
+ WARN_ON(tty->buf.head != tty->buf.tail);
+ tty->buf.head->read = tty->buf.head->commit;
}
/**
@@ -185,19 +188,25 @@ static struct tty_buffer *tty_buffer_find(struct tty_struct *tty, size_t size)
/* Should possibly check if this fails for the largest buffer we
have queued and recycle that ? */
}
+
/**
- * __tty_buffer_request_room - grow tty buffer if needed
+ * tty_buffer_request_room - grow tty buffer if needed
* @tty: tty structure
* @size: size desired
*
* Make at least size bytes of linear space available for the tty
* buffer. If we fail return the size we managed to find.
- * Locking: Caller must hold tty->buf.lock
+ *
+ * Locking: Takes tty->buf.lock
*/
-static int __tty_buffer_request_room(struct tty_struct *tty, size_t size)
+int tty_buffer_request_room(struct tty_struct *tty, size_t size)
{
struct tty_buffer *b, *n;
int left;
+ unsigned long flags;
+
+ spin_lock_irqsave(&tty->buf.lock, flags);
+
/* OPTIMISATION: We could keep a per tty "zero" sized buffer to
remove this conditional if its worth it. This would be invisible
to the callers */
@@ -219,29 +228,8 @@ static int __tty_buffer_request_room(struct tty_struct *tty, size_t size)
size = left;
}
- return size;
-}
-
-
-/**
- * tty_buffer_request_room - grow tty buffer if needed
- * @tty: tty structure
- * @size: size desired
- *
- * Make at least size bytes of linear space available for the tty
- * buffer. If we fail return the size we managed to find.
- *
- * Locking: Takes tty->buf.lock
- */
-int tty_buffer_request_room(struct tty_struct *tty, size_t size)
-{
- unsigned long flags;
- int length;
-
- spin_lock_irqsave(&tty->buf.lock, flags);
- length = __tty_buffer_request_room(tty, size);
spin_unlock_irqrestore(&tty->buf.lock, flags);
- return length;
+ return size;
}
EXPORT_SYMBOL_GPL(tty_buffer_request_room);
@@ -264,22 +252,14 @@ int tty_insert_flip_string_fixed_flag(struct tty_struct *tty,
int copied = 0;
do {
int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE);
- int space;
- unsigned long flags;
- struct tty_buffer *tb;
-
- spin_lock_irqsave(&tty->buf.lock, flags);
- space = __tty_buffer_request_room(tty, goal);
- tb = tty->buf.tail;
+ int space = tty_buffer_request_room(tty, goal);
+ struct tty_buffer *tb = tty->buf.tail;
/* If there is no space then tb may be NULL */
- if (unlikely(space == 0)) {
- spin_unlock_irqrestore(&tty->buf.lock, flags);
+ if (unlikely(space == 0))
break;
- }
memcpy(tb->char_buf_ptr + tb->used, chars, space);
memset(tb->flag_buf_ptr + tb->used, flag, space);
tb->used += space;
- spin_unlock_irqrestore(&tty->buf.lock, flags);
copied += space;
chars += space;
/* There is a small chance that we need to split the data over
@@ -309,22 +289,14 @@ int tty_insert_flip_string_flags(struct tty_struct *tty,
int copied = 0;
do {
int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE);
- int space;
- unsigned long __flags;
- struct tty_buffer *tb;
-
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox