[PATCH 1/3] serial: 8250: Remove trailing space in 8250 driver

4 views
Skip to first unread message

Simon Glass

unread,
Jan 17, 2012, 1:56:01 PM1/17/12
to LKML, Greg Kroah-Hartman, linux-...@vger.kernel.org, Simon Glass
Trivial change to remove a trailing space.

Signed-off-by: Simon Glass <s...@chromium.org>
---
drivers/tty/serial/8250.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/tty/serial/8250.c b/drivers/tty/serial/8250.c
index 9f50c4e..54f8920 100644
--- a/drivers/tty/serial/8250.c
+++ b/drivers/tty/serial/8250.c
@@ -865,7 +865,7 @@ static int broken_efr(struct uart_8250_port *up)
/*
* Exar ST16C2550 "A2" devices incorrectly detect as
* having an EFR, and report an ID of 0x0201. See
- * http://linux.derkeiler.com/Mailing-Lists/Kernel/2004-11/4812.html
+ * http://linux.derkeiler.com/Mailing-Lists/Kernel/2004-11/4812.html
*/
if (autoconfig_read_divisor_id(up) == 0x0201 && size_fifo(up) == 16)
return 1;
--
1.7.7.3

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

Simon Glass

unread,
Jan 17, 2012, 1:56:03 PM1/17/12
to LKML, Greg Kroah-Hartman, linux-...@vger.kernel.org, Simon Glass
Since serial_core now does not make serial ports wake-up capable by
default, add a parameter to support this feature in the 8250 UART.
This is the only UART where I think this feature is useful.

Signed-off-by: Simon Glass <s...@chromium.org>
---

drivers/tty/serial/8250.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/tty/serial/8250.c b/drivers/tty/serial/8250.c
index 54f8920..78feee4 100644
--- a/drivers/tty/serial/8250.c
+++ b/drivers/tty/serial/8250.c
@@ -65,6 +65,7 @@ static int serial_index(struct uart_port *port)
}

static unsigned int skip_txen_test; /* force skip of txen test at init time */
+static unsigned int wakeup_capable; /* device can wake up system */

/*
* Debugging.
@@ -2786,6 +2787,7 @@ serial8250_register_ports(struct uart_driver *drv, struct device *dev)

if (up->port.flags & UPF_FIXED_TYPE)
serial8250_init_fixed_type_port(up, up->port.type);
+ up->port.wakeup_capable = wakeup_capable;

uart_add_one_port(drv, &up->port);
}
@@ -3219,6 +3221,7 @@ int serial8250_register_port(struct uart_port *port)
uart->port.set_termios = port->set_termios;
if (port->pm)
uart->port.pm = port->pm;
+ uart->port.wakeup_capable = wakeup_capable;

if (serial8250_isa_config != NULL)
serial8250_isa_config(0, &uart->port,
@@ -3252,6 +3255,7 @@ void serial8250_unregister_port(int line)
uart->port.type = PORT_UNKNOWN;
uart->port.dev = &serial8250_isa_devs->dev;
uart->capabilities = uart_config[uart->port.type].flags;
+ uart->port.wakeup_capable = wakeup_capable;
uart_add_one_port(&serial8250_reg, &uart->port);
} else {
uart->port.dev = NULL;
@@ -3355,3 +3359,6 @@ module_param_array(probe_rsa, ulong, &probe_rsa_count, 0444);
MODULE_PARM_DESC(probe_rsa, "Probe I/O ports for RSA");
#endif
MODULE_ALIAS_CHARDEV_MAJOR(TTY_MAJOR);
+
+module_param(wakeup_capable, uint, 0644);
+MODULE_PARM_DESC(wakeup_capable, "Allow driver to wake up the system");

Simon Glass

unread,
Jan 17, 2012, 1:56:02 PM1/17/12
to LKML, Greg Kroah-Hartman, linux-...@vger.kernel.org, Simon Glass
The synchronize_rcu() call resulting from making every serial driver
wake-up capable (commit b3b708fa) slows boot down by 600ms on my Tegra2x
system (with CONFIG_PREEMPT disabled).

Waking up the machine from a serial port seems to be an unlikely
requirement, so make serial_core default to not using the behaviour.

Before:
[ 0.338809] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
[ 0.951861] serial8250.0: ttyS0 at MMIO 0x70006040 (irq = 69) is a Tegra
[ 1.666042] console [ttyS0] enabled

After:
[ 0.338452] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
[ 0.339016] serial8250.0: ttyS0 at MMIO 0x70006040 (irq = 69) is a Tegra
[ 1.041860] console [ttyS0] enabled

Signed-off-by: Simon Glass <s...@chromium.org>
---

drivers/tty/serial/serial_core.c | 6 ++++--
include/linux/serial_core.h | 3 ++-
2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index c7bf31a..0129551 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2348,8 +2348,10 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport)
*/
tty_dev = tty_register_device(drv->tty_driver, uport->line, uport->dev);
if (likely(!IS_ERR(tty_dev))) {
- device_init_wakeup(tty_dev, 1);
- device_set_wakeup_enable(tty_dev, 0);
+ if (uport->wakeup_capable) {
+ device_init_wakeup(tty_dev, 1);
+ device_set_wakeup_enable(tty_dev, 0);
+ }
} else
printk(KERN_ERR "Cannot register tty device on line %d\n",
uport->line);
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index c91ace7..9a87975 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -377,7 +377,8 @@ struct uart_port {
unsigned char hub6; /* this should be in the 8250 driver */
unsigned char suspended;
unsigned char irq_wake;
- unsigned char unused[2];
+ unsigned char wakeup_capable; /* 1 if wake-up capable */
+ unsigned char unused;
void *private_data; /* generic platform data pointer */

Alan Cox

unread,
Jan 17, 2012, 3:09:42 PM1/17/12
to Simon Glass, LKML, Greg Kroah-Hartman, linux-...@vger.kernel.org
On Tue, 17 Jan 2012 10:56:02 -0800
Simon Glass <s...@chromium.org> wrote:

> The synchronize_rcu() call resulting from making every serial driver
> wake-up capable (commit b3b708fa) slows boot down by 600ms on my Tegra2x
> system (with CONFIG_PREEMPT disabled).
>
> Waking up the machine from a serial port seems to be an unlikely
> requirement, so make serial_core default to not using the behaviour.

Its quite normal on x86, and very very common on embedded. Sorry someone
needs to fix the root cause not hack the driver.

The other question would be whether in PC space you can deduce the wake
up situation from ACPI data ? One for Len ?

Alan

Alan Cox

unread,
Jan 17, 2012, 3:10:36 PM1/17/12
to Simon Glass, LKML, Greg Kroah-Hartman, linux-...@vger.kernel.org
On Tue, 17 Jan 2012 10:56:03 -0800
Simon Glass <s...@chromium.org> wrote:

> Since serial_core now does not make serial ports wake-up capable by
> default, add a parameter to support this feature in the 8250 UART.
> This is the only UART where I think this feature is useful.

NAK

Things should just work for users. Magic parameters is not an
improvement. If its a performance problem someone needs to fix the rcu
sync overhead or stop using rcu on that path.

Alan

Paul E. McKenney

unread,
Jan 17, 2012, 11:17:21 PM1/17/12
to Alan Cox, Simon Glass, LKML, Greg Kroah-Hartman, linux-...@vger.kernel.org
On Tue, Jan 17, 2012 at 08:10:36PM +0000, Alan Cox wrote:
> On Tue, 17 Jan 2012 10:56:03 -0800
> Simon Glass <s...@chromium.org> wrote:
>
> > Since serial_core now does not make serial ports wake-up capable by
> > default, add a parameter to support this feature in the 8250 UART.
> > This is the only UART where I think this feature is useful.
>
> NAK
>
> Things should just work for users. Magic parameters is not an
> improvement. If its a performance problem someone needs to fix the rcu
> sync overhead or stop using rcu on that path.

I must say that I lack context here, even after looking at the patch,
but the synchronize_rcu_expedited() primitives can be used if the latency
of synchronize_rcu() is too large.

Thanx, Paul

Reply all
Reply to author
Forward
0 new messages