>From the patch, pm_runtime_get_sync() will be called in interrupt context and spin_lock context.
1. pm_runtime_get_sync() called in interrupt handler.
File: drivers/tty/serial/8250/8250_core.c
void serial8250_rpm_get(struct uart_8250_port *p)
{
if (!(p->capabilities & UART_CAP_RPM))
return;
pm_runtime_get_sync(p->port.dev);
}
static int serial8250_default_handle_irq(struct uart_port *port)
{
struct uart_8250_port *up = up_to_u8250p(port);
unsigned int iir;
int ret;
serial8250_rpm_get(up);
iir = serial_port_in(port, UART_IIR);
ret = serial8250_handle_irq(port, iir);
serial8250_rpm_put(up);
return ret;
}
2. pm_runtime_get_sync() called in spin_lock context.
File: drivers/tty/serial/8250/8250_core.c
static void serial8250_start_tx(struct uart_port *port)
{
struct uart_8250_port *up = up_to_u8250p(port);
serial8250_rpm_get_tx(up);
if (up->dma && !up->dma->tx_dma(up)) {
return;
} else if (!(up->ier & UART_IER_THRI)) {
up->ier |= UART_IER_THRI;
serial_port_out(port, UART_IER, up->ier);
......
File: drivers/tty/serial/serial_core.c
static void __uart_start(struct tty_struct *tty)
{
struct uart_state *state = tty->driver_data;
struct uart_port *port = state->uart_port;
if (port->ops->wake_peer)
port->ops->wake_peer(port);
if (!uart_tx_stopped(port))
port->ops->start_tx(port);
}
static void uart_start(struct tty_struct *tty)
{
struct uart_state *state = tty->driver_data;
struct uart_port *port = state->uart_port;
unsigned long flags;
spin_lock_irqsave(&port->lock, flags);
__uart_start(tty);
spin_unlock_irqrestore(&port->lock, flags);
}
As we know pm_runtime_get_sync() may have sleep function, if call in spin_lock context and interrupt context, will cause lots of issue.
Anyone have some idea on this, how to fix and implement RPM mechanism?
Thanks,
Huiquan
-----Original Message-----
From: linux-serial-owner@xxxxxxxxxxxxxxx [mailto:linux-serial-owner@xxxxxxxxxxxxxxx] On Behalf Of Zhong, Huiquan
Sent: Wednesday, March 11, 2015 9:50 AM
To: Tony Lindgren
Cc: Sebastian Andrzej Siewior; linux-serial@xxxxxxxxxxxxxxx; mika.westerberg@xxxxxxxxxxxxxxx; alan@xxxxxxxxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx
Subject: RE: tty: serial: 8250_core runtime pm issue
Yes, So is it ok for the patch to enable RPM? there are lots of limitation for runtime pm callback.
pm_runtime_get_sync() may have sleep function, if call in spin_lock context and interrupt context, will cause lots of issue.
And pm_runtime_irq_safe() can only ensure interrupt is safe, but spin_lock still is a question.
Anyone have some idea about enable RPM with 8250 driver?
>From 38c9e55dacc62022289f5f4e2ccae120eb8a5e1d Mon Sep 17 00:00:00 2001
From: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
Date: Wed, 10 Sep 2014 21:29:57 +0200
Subject: [PATCH] tty: serial: 8250_core: add run time pm
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
While comparing the OMAP-serial and the 8250 part of this I noticed that the latter does not use run time-pm. Here are the pieces. It is basically a get before first register access and a last_busy + put after last access. This has to be enabled from userland _and_ UART_CAP_RPM is required for this.
The runtime PM can usually work transparently in the background however there is one exception to this: After serial8250_tx_chars() completes there still may be unsent bytes in the FIFO (depending on CPU speed vs baud rate + flow control). Even if the TTY-buffer is empty we do not want RPM to disable the device because it won't send the remaining bytes. Instead we leave serial8250_tx_chars() with RPM enabled and wait for the FIFO empty interrupt. Once we enter serial8250_tx_chars() with an empty buffer we know that the FIFO is empty and since we are not going to send anything, we can disable the device.
That xchg() is to ensure that serial8250_tx_chars() can be called multiple times and only the first invocation will actually invoke the runtime PM function. So that the last invocation of __stop_tx() will disable runtime pm.
NOTE: do not enable RPM on the device unless you know what you do! If the device goes idle, it won't be woken up by incomming RX data _unless_ there is a wakeup irq configured which is usually the RX pin configure for wakeup via the reset module. The RX activity will then wake up the device from idle. However the first character is garbage and lost. The following bytes will be received once the device is up in time. On the beagle board xm (omap3) it takes approx 13ms from the first wakeup byte until the first byte that is received properly if the device was in core-off.
v5...v8:
- drop RPM from serial8250_set_mctrl() it will be used in
restore path which already has RPM active and holds
dev->power.lock
v4...v5:
- add a wrapper around rpm function and introduce UART_CAP_RPM
to ensure RPM put is invoked after the TX FIFO is empty.
v3...v4:
- added runtime to the console code
- removed device_may_wakeup() from serial8250_set_sleep()
Change-Id: I4b70e325d616de5fbcc11fdff3c0f3835ce99341
Cc: mika.westerberg@xxxxxxxxxxxxxxx
Reviewed-by: Tony Lindgren <tony@xxxxxxxxxxx>
Tested-by: Tony Lindgren <tony@xxxxxxxxxxx>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
Thanks,
Huiquan
-----Original Message-----
From: Tony Lindgren [mailto:tony@xxxxxxxxxxx]
Sent: Wednesday, March 11, 2015 1:43 AM
To: Zhong, Huiquan
Cc: Sebastian Andrzej Siewior; linux-serial@xxxxxxxxxxxxxxx; mika.westerberg@xxxxxxxxxxxxxxx; alan@xxxxxxxxxxxxxxx
Subject: Re: tty: serial: 8250_core runtime pm issue
* Zhong, Huiquan <huiquan.zhong@xxxxxxxxx> [150309 19:27]:
> Yes, I add pm_runtime_irq_safe() too, but in 8250_dw.c there are sleep() in intel lpss runtime_resume function, So if using spin_lock, there are lots of issue.
>
> That is what I want said, we can't restrict there are no sleep function in runtime PM callback.
Yes agreed, we should not need to use pm_runtime_irq_safe() in general for drivers at all. Having pm_runtime calls in the irq handler probably means the driver should just do the handling in a bottom half anyways.
In many cases the driver does need to sleep on resume to because of the need to start up regulators.
Regards,
Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
