* [PATCH V2 2/2] tty: serial: qcom_geni_serial: Wakeup over UART RX
@ 2019-10-10 9:46 Akash Asthana
2019-10-10 14:25 ` Stephen Boyd
0 siblings, 1 reply; 7+ messages in thread
From: Akash Asthana @ 2019-10-10 9:46 UTC (permalink / raw)
To: gregkh
Cc: linux-arm-msm, linux-serial, mgautam, bjorn.andersson, swboyd,
Akash Asthana
Add system wakeup capability over UART RX line for wakeup capable UART.
When system is suspended, RX line act as an interrupt to wakeup system
for any communication requests from peer.
Signed-off-by: Akash Asthana <akashast@codeaurora.org>
---
Changes in v2:
- Split v1 patch in two.
- Address review comments v1 patch.
drivers/tty/serial/qcom_geni_serial.c | 44 ++++++++++++++++++++++++++++++++++-
1 file changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 5180cd8..ff63728 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -14,6 +14,7 @@
#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/platform_device.h>
+#include <linux/pm_wakeirq.h>
#include <linux/qcom-geni-se.h>
#include <linux/serial.h>
#include <linux/serial_core.h>
@@ -116,6 +117,7 @@ struct qcom_geni_serial_port {
bool brk;
unsigned int tx_remaining;
+ int wakeup_irq;
};
static const struct uart_ops qcom_geni_console_pops;
@@ -755,6 +757,15 @@ static void qcom_geni_serial_handle_tx(struct uart_port *uport, bool done,
uart_write_wakeup(uport);
}
+static irqreturn_t qcom_geni_serial_wakeup_isr(int isr, void *dev)
+{
+ struct uart_port *uport = dev;
+
+ pm_wakeup_event(uport->dev, 2000);
+
+ return IRQ_HANDLED;
+}
+
static irqreturn_t qcom_geni_serial_isr(int isr, void *dev)
{
u32 m_irq_en;
@@ -1306,6 +1317,29 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
return ret;
}
+ if (!console) {
+ port->wakeup_irq = platform_get_irq(pdev, 1);
+ if (port->wakeup_irq < 0) {
+ dev_err(&pdev->dev, "Failed to get wakeup IRQ %d\n",
+ port->wakeup_irq);
+ } else {
+ irq_set_status_flags(port->wakeup_irq, IRQ_NOAUTOEN);
+ ret = devm_request_irq(uport->dev, port->wakeup_irq,
+ qcom_geni_serial_wakeup_isr,
+ IRQF_TRIGGER_FALLING, "uart_wakeup", uport);
+ if (ret) {
+ dev_err(uport->dev, "Failed to register wakeup IRQ ret %d\n",
+ ret);
+ return ret;
+ }
+
+ device_init_wakeup(&pdev->dev, true);
+ ret = dev_pm_set_wake_irq(&pdev->dev, port->wakeup_irq);
+ if (unlikely(ret))
+ dev_err(uport->dev, "%s:Failed to set IRQ wake:%d\n",
+ __func__, ret);
+ }
+ }
uport->private_data = drv;
platform_set_drvdata(pdev, port);
port->handle_rx = console ? handle_rx_console : handle_rx_uart;
@@ -1328,7 +1362,12 @@ static int __maybe_unused qcom_geni_serial_sys_suspend(struct device *dev)
struct qcom_geni_serial_port *port = dev_get_drvdata(dev);
struct uart_port *uport = &port->uport;
- return uart_suspend_port(uport->private_data, uport);
+ uart_suspend_port(uport->private_data, uport);
+
+ if (port->wakeup_irq > 0)
+ enable_irq(port->wakeup_irq);
+
+ return 0;
}
static int __maybe_unused qcom_geni_serial_sys_resume(struct device *dev)
@@ -1336,6 +1375,9 @@ static int __maybe_unused qcom_geni_serial_sys_resume(struct device *dev)
struct qcom_geni_serial_port *port = dev_get_drvdata(dev);
struct uart_port *uport = &port->uport;
+ if (port->wakeup_irq > 0)
+ disable_irq(port->wakeup_irq);
+
return uart_resume_port(uport->private_data, uport);
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH V2 2/2] tty: serial: qcom_geni_serial: Wakeup over UART RX
2019-10-10 9:46 [PATCH V2 2/2] tty: serial: qcom_geni_serial: Wakeup over UART RX Akash Asthana
@ 2019-10-10 14:25 ` Stephen Boyd
2019-10-11 9:48 ` Akash Asthana
0 siblings, 1 reply; 7+ messages in thread
From: Stephen Boyd @ 2019-10-10 14:25 UTC (permalink / raw)
To: Akash Asthana, gregkh
Cc: linux-arm-msm, linux-serial, mgautam, bjorn.andersson,
Akash Asthana
Quoting Akash Asthana (2019-10-10 02:46:43)
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 5180cd8..ff63728 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -1306,6 +1317,29 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
> return ret;
> }
>
> + if (!console) {
> + port->wakeup_irq = platform_get_irq(pdev, 1);
Should use platform_get_irq_optional() it seems.
> + if (port->wakeup_irq < 0) {
> + dev_err(&pdev->dev, "Failed to get wakeup IRQ %d\n",
> + port->wakeup_irq);
> + } else {
> + irq_set_status_flags(port->wakeup_irq, IRQ_NOAUTOEN);
> + ret = devm_request_irq(uport->dev, port->wakeup_irq,
> + qcom_geni_serial_wakeup_isr,
> + IRQF_TRIGGER_FALLING, "uart_wakeup", uport);
> + if (ret) {
> + dev_err(uport->dev, "Failed to register wakeup IRQ ret %d\n",
> + ret);
> + return ret;
> + }
> +
> + device_init_wakeup(&pdev->dev, true);
> + ret = dev_pm_set_wake_irq(&pdev->dev, port->wakeup_irq);
Why can't we use dev_pm_set_dedicated_wake_irq() here?
> + if (unlikely(ret))
> + dev_err(uport->dev, "%s:Failed to set IRQ wake:%d\n",
> + __func__, ret);
> + }
> + }
> uport->private_data = drv;
> platform_set_drvdata(pdev, port);
> port->handle_rx = console ? handle_rx_console : handle_rx_uart;
> @@ -1328,7 +1362,12 @@ static int __maybe_unused qcom_geni_serial_sys_suspend(struct device *dev)
> struct qcom_geni_serial_port *port = dev_get_drvdata(dev);
> struct uart_port *uport = &port->uport;
>
> - return uart_suspend_port(uport->private_data, uport);
> + uart_suspend_port(uport->private_data, uport);
> +
> + if (port->wakeup_irq > 0)
> + enable_irq(port->wakeup_irq);
> +
Then this is hopefully done automatically?
> + return 0;
> }
>
> static int __maybe_unused qcom_geni_serial_sys_resume(struct device *dev)
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH V2 2/2] tty: serial: qcom_geni_serial: Wakeup over UART RX
2019-10-10 14:25 ` Stephen Boyd
@ 2019-10-11 9:48 ` Akash Asthana
2019-10-15 20:10 ` Stephen Boyd
0 siblings, 1 reply; 7+ messages in thread
From: Akash Asthana @ 2019-10-11 9:48 UTC (permalink / raw)
To: Stephen Boyd, gregkh
Cc: linux-arm-msm, linux-serial, mgautam, bjorn.andersson
On 10/10/2019 7:55 PM, Stephen Boyd wrote:
> Quoting Akash Asthana (2019-10-10 02:46:43)
>> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
>> index 5180cd8..ff63728 100644
>> --- a/drivers/tty/serial/qcom_geni_serial.c
>> +++ b/drivers/tty/serial/qcom_geni_serial.c
>> @@ -1306,6 +1317,29 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
>> return ret;
>> }
>>
>> + if (!console) {
>> + port->wakeup_irq = platform_get_irq(pdev, 1);
> Should use platform_get_irq_optional() it seems.
OK
>
>> + if (port->wakeup_irq < 0) {
>> + dev_err(&pdev->dev, "Failed to get wakeup IRQ %d\n",
>> + port->wakeup_irq);
>> + } else {
>> + irq_set_status_flags(port->wakeup_irq, IRQ_NOAUTOEN);
>> + ret = devm_request_irq(uport->dev, port->wakeup_irq,
>> + qcom_geni_serial_wakeup_isr,
>> + IRQF_TRIGGER_FALLING, "uart_wakeup", uport);
>> + if (ret) {
>> + dev_err(uport->dev, "Failed to register wakeup IRQ ret %d\n",
>> + ret);
>> + return ret;
>> + }
>> +
>> + device_init_wakeup(&pdev->dev, true);
>> + ret = dev_pm_set_wake_irq(&pdev->dev, port->wakeup_irq);
> Why can't we use dev_pm_set_dedicated_wake_irq() here?
If we use this API then handler "handle_threaded_wake_irq" uses device
specific pm_runtime functions to wake the device and currently this
driver don't support runtime PM callbacks. Also, we want to register
"qcom_geni_serial_wakeup_isr" as our IRQ handler for wakeup scenario.
>
>> + if (unlikely(ret))
>> + dev_err(uport->dev, "%s:Failed to set IRQ wake:%d\n",
>> + __func__, ret);
>> + }
>> + }
>> uport->private_data = drv;
>> platform_set_drvdata(pdev, port);
>> port->handle_rx = console ? handle_rx_console : handle_rx_uart;
>> @@ -1328,7 +1362,12 @@ static int __maybe_unused qcom_geni_serial_sys_suspend(struct device *dev)
>> struct qcom_geni_serial_port *port = dev_get_drvdata(dev);
>> struct uart_port *uport = &port->uport;
>>
>> - return uart_suspend_port(uport->private_data, uport);
>> + uart_suspend_port(uport->private_data, uport);
>> +
>> + if (port->wakeup_irq > 0)
>> + enable_irq(port->wakeup_irq);
>> +
> Then this is hopefully done automatically?
>
>> + return 0;
>> }
>>
>> static int __maybe_unused qcom_geni_serial_sys_resume(struct device *dev)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH V2 2/2] tty: serial: qcom_geni_serial: Wakeup over UART RX
2019-10-11 9:48 ` Akash Asthana
@ 2019-10-15 20:10 ` Stephen Boyd
2019-10-17 11:10 ` Akash Asthana
0 siblings, 1 reply; 7+ messages in thread
From: Stephen Boyd @ 2019-10-15 20:10 UTC (permalink / raw)
To: Akash Asthana, gregkh
Cc: linux-arm-msm, linux-serial, mgautam, bjorn.andersson
Quoting Akash Asthana (2019-10-11 02:48:42)
>
> On 10/10/2019 7:55 PM, Stephen Boyd wrote:
> > Quoting Akash Asthana (2019-10-10 02:46:43)
> >> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> >> index 5180cd8..ff63728 100644
> >> --- a/drivers/tty/serial/qcom_geni_serial.c
> >> +++ b/drivers/tty/serial/qcom_geni_serial.c
> >> @@ -1306,6 +1317,29 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
> >> + if (port->wakeup_irq < 0) {
> >> + dev_err(&pdev->dev, "Failed to get wakeup IRQ %d\n",
> >> + port->wakeup_irq);
> >> + } else {
> >> + irq_set_status_flags(port->wakeup_irq, IRQ_NOAUTOEN);
> >> + ret = devm_request_irq(uport->dev, port->wakeup_irq,
> >> + qcom_geni_serial_wakeup_isr,
> >> + IRQF_TRIGGER_FALLING, "uart_wakeup", uport);
> >> + if (ret) {
> >> + dev_err(uport->dev, "Failed to register wakeup IRQ ret %d\n",
> >> + ret);
> >> + return ret;
> >> + }
> >> +
> >> + device_init_wakeup(&pdev->dev, true);
> >> + ret = dev_pm_set_wake_irq(&pdev->dev, port->wakeup_irq);
> > Why can't we use dev_pm_set_dedicated_wake_irq() here?
>
> If we use this API then handler "handle_threaded_wake_irq" uses device
> specific pm_runtime functions to wake the device and currently this
>
> driver don't support runtime PM callbacks. Also, we want to register
> "qcom_geni_serial_wakeup_isr" as our IRQ handler for wakeup scenario.
>
Why can't we make this driver use runtime PM?
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH V2 2/2] tty: serial: qcom_geni_serial: Wakeup over UART RX
2019-10-15 20:10 ` Stephen Boyd
@ 2019-10-17 11:10 ` Akash Asthana
2019-10-28 15:00 ` Stephen Boyd
0 siblings, 1 reply; 7+ messages in thread
From: Akash Asthana @ 2019-10-17 11:10 UTC (permalink / raw)
To: Stephen Boyd, gregkh
Cc: linux-arm-msm, linux-serial, mgautam, bjorn.andersson
On 10/16/2019 1:40 AM, Stephen Boyd wrote:
> Quoting Akash Asthana (2019-10-11 02:48:42)
>> On 10/10/2019 7:55 PM, Stephen Boyd wrote:
>>> Quoting Akash Asthana (2019-10-10 02:46:43)
>>>> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
>>>> index 5180cd8..ff63728 100644
>>>> --- a/drivers/tty/serial/qcom_geni_serial.c
>>>> +++ b/drivers/tty/serial/qcom_geni_serial.c
>>>> @@ -1306,6 +1317,29 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
>>>> + if (port->wakeup_irq < 0) {
>>>> + dev_err(&pdev->dev, "Failed to get wakeup IRQ %d\n",
>>>> + port->wakeup_irq);
>>>> + } else {
>>>> + irq_set_status_flags(port->wakeup_irq, IRQ_NOAUTOEN);
>>>> + ret = devm_request_irq(uport->dev, port->wakeup_irq,
>>>> + qcom_geni_serial_wakeup_isr,
>>>> + IRQF_TRIGGER_FALLING, "uart_wakeup", uport);
>>>> + if (ret) {
>>>> + dev_err(uport->dev, "Failed to register wakeup IRQ ret %d\n",
>>>> + ret);
>>>> + return ret;
>>>> + }
>>>> +
>>>> + device_init_wakeup(&pdev->dev, true);
>>>> + ret = dev_pm_set_wake_irq(&pdev->dev, port->wakeup_irq);
>>> Why can't we use dev_pm_set_dedicated_wake_irq() here?
>> If we use this API then handler "handle_threaded_wake_irq" uses device
>> specific pm_runtime functions to wake the device and currently this
>>
>> driver don't support runtime PM callbacks. Also, we want to register
>> "qcom_geni_serial_wakeup_isr" as our IRQ handler for wakeup scenario.
>>
> Why can't we make this driver use runtime PM?
Currently there are no plans to use runtime PM as we are interested in
enabling wakeup irq as part of system suspend only.
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH V2 2/2] tty: serial: qcom_geni_serial: Wakeup over UART RX
2019-10-17 11:10 ` Akash Asthana
@ 2019-10-28 15:00 ` Stephen Boyd
2019-11-05 9:56 ` Akash Asthana
0 siblings, 1 reply; 7+ messages in thread
From: Stephen Boyd @ 2019-10-28 15:00 UTC (permalink / raw)
To: Akash Asthana, gregkh
Cc: linux-arm-msm, linux-serial, mgautam, bjorn.andersson
Quoting Akash Asthana (2019-10-17 04:10:10)
>
> On 10/16/2019 1:40 AM, Stephen Boyd wrote:
> > Why can't we make this driver use runtime PM?
>
> Currently there are no plans to use runtime PM as we are interested in
> enabling wakeup irq as part of system suspend only.
>
>
Does the wakeup irq code require runtime PM? I thought that any wake irq
attached to a device is armed during system wide suspend and disabled on
resume. See device_wakeup_arm_wake_irqs() called from
dpm_suspend_noirq().
So why can't we use the common code that manages wakeup irqs for
devices?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V2 2/2] tty: serial: qcom_geni_serial: Wakeup over UART RX
2019-10-28 15:00 ` Stephen Boyd
@ 2019-11-05 9:56 ` Akash Asthana
0 siblings, 0 replies; 7+ messages in thread
From: Akash Asthana @ 2019-11-05 9:56 UTC (permalink / raw)
To: Stephen Boyd, gregkh
Cc: linux-arm-msm, linux-serial, mgautam, bjorn.andersson
On 10/28/2019 8:30 PM, Stephen Boyd wrote:
> Quoting Akash Asthana (2019-10-17 04:10:10)
>> On 10/16/2019 1:40 AM, Stephen Boyd wrote:
>>> Why can't we make this driver use runtime PM?
>> Currently there are no plans to use runtime PM as we are interested in
>> enabling wakeup irq as part of system suspend only.
>>
>>
> Does the wakeup irq code require runtime PM? I thought that any wake irq
> attached to a device is armed during system wide suspend and disabled on
> resume. See device_wakeup_arm_wake_irqs() called from
> dpm_suspend_noirq().
>
> So why can't we use the common code that manages wakeup irqs for
> devices?
After reading about device_wakeup_arm_wake_irqs() and
dpm_suspend_noirq() APIs it's clear to me
that we don't require runtime PM feature. We have now aligned our driver
to use common code that manages
wakeup irqs for devices.
Thanks for suggesting this changes!
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-11-05 9:56 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-10-10 9:46 [PATCH V2 2/2] tty: serial: qcom_geni_serial: Wakeup over UART RX Akash Asthana
2019-10-10 14:25 ` Stephen Boyd
2019-10-11 9:48 ` Akash Asthana
2019-10-15 20:10 ` Stephen Boyd
2019-10-17 11:10 ` Akash Asthana
2019-10-28 15:00 ` Stephen Boyd
2019-11-05 9:56 ` Akash Asthana
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).