|
|
| [Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] |
"Govindraj.R" <govindraj.raja@xxxxxx> writes:
> From: "Govindraj.R" <govindraj.raja@xxxxxx>
>
> With set_wakeup port ops availability from serial_core layer
> which will be called when port is opened with state as true
> indicating the wakeups can be enabled for this port and state
> as false while port shutdown where we can disable the wakeups.
>
> But wakeup can be also enabled/disabled when requested from sysfs.
> If disabled wakeup will be disabled while entering suspend and be
> enabled back based on pm state while resuming.
Nice, this is definitely the right direction. Thanks.
> Thanks to Kevin Hilman <khilman@xxxxxx> for suggesting this.
> Discussion References:
> http://www.spinics.net/lists/linux-omap/msg67764.html
> http://www.spinics.net/lists/linux-omap/msg67838.html
>
> Cc: Paul Walmsley <paul@xxxxxxxxx>
> Signed-off-by: Kevin Hilman <khilman@xxxxxx>
> [suggestion and modification to enable and disable wakeup
> capability based on port usage]
I haven't signed-off on this, and technically since I'm not on the
delivery path, I shouldn't have a sign-off either. See
Documenation/SubmittingPatches for all the details on s-o-b tags.
I will give it an ack or tested-by after I've gone through it, but for
now I have a few minor comments below. Also just reported that l-o
master has broken UART wakeups due to the default mux settings change
earlier, so that has to be sorted out in order to properly test this.
> Signed-off-by: Govindraj.R <govindraj.raja@xxxxxx>
> ---
>
> Patch is tested using the patch as in here[1]
> Tested on beagle-XM by enabling and disabling wakeups
> from sysfs and opening and closing a uart port.
>
> [1]: http://marc.info/?l=linux-omap&m=133518614022144&w=2
>
>
> arch/arm/plat-omap/include/plat/omap-serial.h | 1 -
> drivers/tty/serial/omap-serial.c | 45 +++++++++++++++----------
> 2 files changed, 27 insertions(+), 19 deletions(-)
>
> diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h
> index 9ff4444..8e6d734 100644
> --- a/arch/arm/plat-omap/include/plat/omap-serial.h
> +++ b/arch/arm/plat-omap/include/plat/omap-serial.h
> @@ -130,7 +130,6 @@ struct uart_omap_port {
> unsigned long port_activity;
> u32 context_loss_cnt;
> u32 errata;
> - u8 wakeups_enabled;
>
> struct pm_qos_request pm_qos_request;
> u32 latency;
> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
> index d00b38e..b8f328e 100644
> --- a/drivers/tty/serial/omap-serial.c
> +++ b/drivers/tty/serial/omap-serial.c
> @@ -525,6 +525,7 @@ static int serial_omap_startup(struct uart_port *port)
> dev_dbg(up->port.dev, "serial_omap_startup+%d\n", up->port.line);
>
> pm_runtime_get_sync(&up->pdev->dev);
> +
stray whitespace change?
> /*
> * Clear the FIFO buffers and disable them.
> * (they will be reenabled in set_termios())
> @@ -910,11 +911,27 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
> dev_dbg(up->port.dev, "serial_omap_set_termios+%d\n", up->port.line);
> }
>
> +static int serial_omap_set_wake(struct uart_port *port, unsigned int state)
> +{
> + struct uart_omap_port *up = (struct uart_omap_port *)port;
> + struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
> + u8 enable_wakeup = false;
s/u8/bool/
> +
> + if (state)
> + enable_wakeup = true;
> +
> + if (pdata->enable_wakeup)
> + pdata->enable_wakeup(up->pdev, enable_wakeup);
but actually, the above 7 lines could be more concisely written:
if (pdata->enable_wakeup)
pdata->enable_wakeup(up->pdev, state ? true : false);
> +
> + return 0;
> +}
> +
> static void
> serial_omap_pm(struct uart_port *port, unsigned int state,
> unsigned int oldstate)
> {
> struct uart_omap_port *up = (struct uart_omap_port *)port;
> + struct omap_uart_port_info *pdata = up->pdev->dev.platform_data;
> unsigned char efr;
>
> dev_dbg(up->port.dev, "serial_omap_pm+%d\n", up->port.line);
> @@ -930,12 +947,8 @@ serial_omap_pm(struct uart_port *port, unsigned int state,
> serial_out(up, UART_EFR, efr);
> serial_out(up, UART_LCR, 0);
>
> - if (!device_may_wakeup(&up->pdev->dev)) {
> - if (!state)
> - pm_runtime_forbid(&up->pdev->dev);
> - else
> - pm_runtime_allow(&up->pdev->dev);
> - }
> + if (!state && pdata->enable_wakeup)
> + pdata->enable_wakeup(up->pdev, true);
>
> pm_runtime_put(&up->pdev->dev);
> }
> @@ -1161,6 +1174,7 @@ static struct uart_ops serial_omap_pops = {
> .shutdown = serial_omap_shutdown,
> .set_termios = serial_omap_set_termios,
> .pm = serial_omap_pm,
> + .set_wake = serial_omap_set_wake,
> .type = serial_omap_type,
> .release_port = serial_omap_release_port,
> .request_port = serial_omap_request_port,
> @@ -1184,10 +1198,14 @@ static struct uart_driver serial_omap_reg = {
> static int serial_omap_suspend(struct device *dev)
> {
> struct uart_omap_port *up = dev_get_drvdata(dev);
> + struct omap_uart_port_info *pdata = dev->platform_data;
>
> if (up) {
> uart_suspend_port(&serial_omap_reg, &up->port);
> flush_work_sync(&up->qos_work);
> +
> + if (!device_may_wakeup(dev))
> + pdata->enable_wakeup(up->pdev, false);
If wakeups are disabled in suspend, where are they re-enabled? I don't
see anything added to the ->resume() method.
Is serial_omap_pm() called during resume to ensure wakeups are
(re)enabled?
If so this should be well described in the changelog.
Kevin
> }
>
> return 0;
> @@ -1476,6 +1494,9 @@ static int serial_omap_probe(struct platform_device *pdev)
> if (ret != 0)
> goto err_add_port;
>
> + if (omap_up_info->enable_wakeup)
> + omap_up_info->enable_wakeup(pdev, false);
> +
> pm_runtime_put(&pdev->dev);
> platform_set_drvdata(pdev, up);
> return 0;
> @@ -1582,18 +1603,6 @@ static int serial_omap_runtime_suspend(struct device *dev)
> if (pdata->get_context_loss_count)
> up->context_loss_cnt = pdata->get_context_loss_count(dev);
>
> - if (device_may_wakeup(dev)) {
> - if (!up->wakeups_enabled) {
> - pdata->enable_wakeup(up->pdev, true);
> - up->wakeups_enabled = true;
> - }
> - } else {
> - if (up->wakeups_enabled) {
> - pdata->enable_wakeup(up->pdev, false);
> - up->wakeups_enabled = false;
> - }
> - }
> -
> /* Errata i291 */
> if (up->use_dma && pdata->set_forceidle &&
> (up->errata & UART_ERRATA_i291_DMA_FORCEIDLE))
--
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
[Kernel Newbies] [Share Photos] [Security] [Netfilter] [Bugtraq] [Linux PPP] [Linux FS] [Photo] [Yosemite] [Yosemite News] [MIPS Linux] [ARM Linux] [Linux Security] [Linux RAID] [Samba] [Video 4 Linux] [Linmodem] [Device Mapper] [Linux Resources]
|