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

Raja, Govindraj
2012-04-23T08:55:57Z

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