Re: [PATCH] tty: serial_core: Add mechanism to identify port closure.

["Raja, Govindraj" <govindraj.raja@ti.com>]

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