| Message ID | 1523302721-19439-2-git-send-email-kramasub@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Andy Gross |
| Headers | show |
On Mon, Apr 09, 2018 at 01:38:34PM -0600, Karthikeyan Ramasubramanian wrote: > * Document reason for newline character counting in console_write > * Document reason for disabling IRQ in the system resume operation > > Signed-off-by: Karthikeyan Ramasubramanian <kramasub@codeaurora.org> > --- > drivers/tty/serial/qcom_geni_serial.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c > index a1b3eb0..1652b1f 100644 > --- a/drivers/tty/serial/qcom_geni_serial.c > +++ b/drivers/tty/serial/qcom_geni_serial.c > @@ -286,6 +286,10 @@ static void qcom_geni_serial_wr_char(struct uart_port *uport, int ch) > u32 bytes_to_send = count; > > for (i = 0; i < count; i++) { > + /* > + * uart_console_write() adds a carriage return for each newline. > + * Account for additional bytes to be written. > + */ > if (s[i] == '\n') > bytes_to_send++; > } > @@ -1103,6 +1107,14 @@ static int __maybe_unused qcom_geni_serial_sys_resume_noirq(struct device *dev) > > if (console_suspend_enabled && uport->suspended) { > uart_resume_port(uport->private_data, uport); > + /* > + * uart_suspend_port() invokes port shutdown which in turn > + * frees the irq. uart_resume_port invokes port startup which > + * performs request_irq. The request_irq auto-enables the IRQ. > + * In addition, resume_noirq implicitly enables the IRQ and > + * leads to an unbalanced IRQ enable warning. Disable the IRQ > + * before returning so that the warning is suppressed. > + */ For the record, the noirq flow is: dpm_resume_noirq() dpm_noirq_resume_devices() for each dev device_resume_noirq() dpm_noirq_end() resume_irqs() for_each_irq_desc resume_irq() > disable_irq(uport->irq); > } > return 0; Reviewed-by: Matthias Kaehlcke <mka@chromium.org> -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoting Matthias Kaehlcke (2018-04-12 14:41:02) > On Mon, Apr 09, 2018 at 01:38:34PM -0600, Karthikeyan Ramasubramanian wrote: > > @@ -1103,6 +1107,14 @@ static int __maybe_unused qcom_geni_serial_sys_resume_noirq(struct device *dev) > > > > if (console_suspend_enabled && uport->suspended) { > > uart_resume_port(uport->private_data, uport); > > + /* > > + * uart_suspend_port() invokes port shutdown which in turn > > + * frees the irq. uart_resume_port invokes port startup which > > + * performs request_irq. The request_irq auto-enables the IRQ. > > + * In addition, resume_noirq implicitly enables the IRQ and > > + * leads to an unbalanced IRQ enable warning. Disable the IRQ > > + * before returning so that the warning is suppressed. > > + */ > > For the record, the noirq flow is: > > dpm_resume_noirq() > dpm_noirq_resume_devices() > for each dev > device_resume_noirq() > > dpm_noirq_end() > resume_irqs() > for_each_irq_desc > resume_irq() > > I'm still curious why we use the noirq variants of the suspend/resume hooks. Why not use the normal suspend/resume hooks here and then drop the whole disable_irq() dance? > > disable_irq(uport->irq); > > } > > return 0; -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 4/17/2018 12:17 AM, Stephen Boyd wrote: > Quoting Matthias Kaehlcke (2018-04-12 14:41:02) >> On Mon, Apr 09, 2018 at 01:38:34PM -0600, Karthikeyan Ramasubramanian wrote: >>> @@ -1103,6 +1107,14 @@ static int __maybe_unused qcom_geni_serial_sys_resume_noirq(struct device *dev) >>> >>> if (console_suspend_enabled && uport->suspended) { >>> uart_resume_port(uport->private_data, uport); >>> + /* >>> + * uart_suspend_port() invokes port shutdown which in turn >>> + * frees the irq. uart_resume_port invokes port startup which >>> + * performs request_irq. The request_irq auto-enables the IRQ. >>> + * In addition, resume_noirq implicitly enables the IRQ and >>> + * leads to an unbalanced IRQ enable warning. Disable the IRQ >>> + * before returning so that the warning is suppressed. >>> + */ >> >> For the record, the noirq flow is: >> >> dpm_resume_noirq() >> dpm_noirq_resume_devices() >> for each dev >> device_resume_noirq() >> >> dpm_noirq_end() >> resume_irqs() >> for_each_irq_desc >> resume_irq() >> >> > > I'm still curious why we use the noirq variants of the suspend/resume > hooks. Why not use the normal suspend/resume hooks here and then drop > the whole disable_irq() dance? In the non-console use-cases, RX line can be used as a wake-up interrupt. Using no_irq variant helps with turning on the serial engine resources quickly and in turn help with reducing the RX latency. > >>> disable_irq(uport->irq); >>> } >>> return 0; Regards, Karthik.
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c index a1b3eb0..1652b1f 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -286,6 +286,10 @@ static void qcom_geni_serial_wr_char(struct uart_port *uport, int ch) u32 bytes_to_send = count; for (i = 0; i < count; i++) { + /* + * uart_console_write() adds a carriage return for each newline. + * Account for additional bytes to be written. + */ if (s[i] == '\n') bytes_to_send++; } @@ -1103,6 +1107,14 @@ static int __maybe_unused qcom_geni_serial_sys_resume_noirq(struct device *dev) if (console_suspend_enabled && uport->suspended) { uart_resume_port(uport->private_data, uport); + /* + * uart_suspend_port() invokes port shutdown which in turn + * frees the irq. uart_resume_port invokes port startup which + * performs request_irq. The request_irq auto-enables the IRQ. + * In addition, resume_noirq implicitly enables the IRQ and + * leads to an unbalanced IRQ enable warning. Disable the IRQ + * before returning so that the warning is suppressed. + */ disable_irq(uport->irq); } return 0;
* Document reason for newline character counting in console_write * Document reason for disabling IRQ in the system resume operation Signed-off-by: Karthikeyan Ramasubramanian <kramasub@codeaurora.org> --- drivers/tty/serial/qcom_geni_serial.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)