All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Raja, Govindraj" <govindraj.raja@ti.com>
To: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: linux-serial@vger.kernel.org, linux-omap@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Alan Cox <alan@linux.intel.com>
Subject: Re: [PATCH] tty: serial_core: Add mechanism to identify port closure.
Date: Mon, 23 Apr 2012 14:25:35 +0530	[thread overview]
Message-ID: <CAMrsUdLaamf9boyMF824iTXdDce0qB3AiUfMiM3a1gL2DcVPqA@mail.gmail.com> (raw)
In-Reply-To: <20120420142740.053b9981@pyramind.ukuu.org.uk>

Hi Alan,

On Fri, Apr 20, 2012 at 6:57 PM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
>> index 9c4c05b..0dc246d 100644
>> --- a/drivers/tty/serial/serial_core.c
>> +++ b/drivers/tty/serial/serial_core.c
>> @@ -1284,6 +1284,8 @@ static void uart_close(struct tty_struct *tty, struct file *filp)
>>               uart_wait_until_sent(tty, uport->timeout);
>>       }
>>
>> +     state->uart_port->closing = true;
>
> So what are the locking rules for this - when is it valid and when can it
> be tested.
>
> This also still seems a hack to me - the core tty code doesn't have
> suspend/resume/open/close confused. The uart layer does, so really the
> uart layer needs untangling. I'm skeptical yet another magic state
> variable is the answer, particularly when the locking model for the
> variable appears undefined.
>
> Teach the uart core code to pass an extra argument indicating whether its
> really doing shutdown/open or merely doing suspend/resume. It's perfectly
> sensible that it tries to use the same helper for both, its broken that
> the called code cannot tell which.
>
> The parameter would also be a local variable which avoids all the locking
> questions.

Yes adding a _state_ local variable from port_shutdown from serial_core layer
and passing the same to low level driver can inform the low level driver.
whether current ops is shutdown or a suspend.

Also, looking into the struct uart_ops.

struct uart_ops {
.
.
        int             (*set_wake)(struct uart_port *, unsigned int state);
.
.
};

Unfortunately set wake is not being used in serial_core.c
So this uart ops can be used to enable wakeups
when port is opened and disable wakeups when port
is closed.

If set_wake is not supposed to be used in this manner I
will fall back to option1 to use state variable for shutdown ops.

tmp patch as in here[1] to use set_wake.

--
Thanks,
Govindraj.R

[1]:

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 9c4c05b..c176ff2 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1426,6 +1426,9 @@ static void uart_port_shutdown(struct tty_port *port)
 	 */
 	uport->ops->shutdown(uport);

+	if (uport->ops->set_wake)
+		uport->ops->set_wake(uport, false);
+
 	/*
 	 * Ensure that the IRQ handler isn't running on another CPU.
 	 */
@@ -1512,6 +1515,9 @@ static int uart_open(struct tty_struct *tty,
struct file *filp)
 		goto err_dec_count;
 	}

+	if (uport->ops->set_wake)
+		uport->ops->set_wake(uport, true);
+
 	/*
 	 * Make sure the device is in D0 state.
 	 */
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2012-04-23  8:55 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-20 10:57 Govindraj.R
2012-04-20 13:27 ` Alan Cox
2012-04-23  8:55   ` Raja, Govindraj [this message]
2012-04-23 10:07     ` Alan Cox

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAMrsUdLaamf9boyMF824iTXdDce0qB3AiUfMiM3a1gL2DcVPqA@mail.gmail.com \
    --to=govindraj.raja@ti.com \
    --cc=alan@linux.intel.com \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --subject='Re: [PATCH] tty: serial_core: Add mechanism to identify port closure.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.