On 3/21/2018 11:20 AM, Stephen Boyd wrote:

Quoting Karthik Ramasubramanian (2018-03-20 15:53:25)

quoted

On 3/20/2018 9:37 AM, Stephen Boyd wrote:

quoted

Quoting Karthikeyan Ramasubramanian (2018-03-14 16:58:49)

quoted

diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
new file mode 100644
index 0000000..1442777
--- /dev/null
+++ b/drivers/tty/serial/qcom_geni_serial.c

@@ -0,0 +1,1158 @@
+

quoted

quoted

Can you also support the OF_EARLYCON_DECLARE method of console writing
so we can get an early printk style debug console?

Do you prefer that as part of this patch itself or is it ok if I upload
the earlycon support once this gets merged.

I think this already got merged? So just split it out into another patch
would be fine. I see the config is already selecting the earlycon
support so it must be planned.

Yes it is definitely in the plan. Since the serial driver got merged, I
will address the pending comments in this patch series. I will upload
the early console support in another patch.

quoted

quoted

quoted

+
+static int __maybe_unused qcom_geni_serial_sys_resume_noirq(struct device *dev)
+{
+       struct platform_device *pdev = to_platform_device(dev);
+       struct qcom_geni_serial_port *port = platform_get_drvdata(pdev);
+       struct uart_port *uport = &port->uport;
+
+       if (console_suspend_enabled && uport->suspended) {
+               uart_resume_port(uport->private_data, uport);
+               disable_irq(uport->irq);

I missed the enable_irq() part. Is this still necessary?

Suspending the uart console port invokes the uart port shutdown
operation. The shutdown operation disables and frees the concerned IRQ.
Resuming the uart console port invokes the uart port startup operation
which requests for the IRQ. The request_irq operation auto-enables the
IRQ. In addition, resume_noirq implicitly enables the IRQ. This leads to
an unbalanced IRQ enable warning.

This disable_irq() helps with suppressing that warning.

That's not obvious so we need a big comment here.

I thought we would move this to the normal suspend/resume callbacks and
skip the noirq variants. That would make this disable_irq() unnecessary
then?

For a non-console UART(eg. 4-wire UART), to reduce the wakeup latency
_noirq variant is used so that the resources can be turned on as soon as
possible. The idea is to use the same suspend and resume operation for
both debug-uart and regular uart. Hence using the _noirq variant.

I will add a detailed comment regarding why we are disabling the IRQ.

BTW, free_irq() should disable the irq itself, so it looks odd to have a
disable_irq() followed directly by a free_irq().

I will update to just free_irq.

Regards,
Karthik.
-- 
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html