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
