Re: [PATCH] usb: gadget: serial: fix Tx stall after buffer overflow

[Sergey Organov <sorganov@gmail.com>]

Michał Mirosław <mirq-linux@rere.qmqm.pl> writes:

> On Mon, Jan 20, 2020 at 09:06:18AM +0300, Sergey Organov wrote:
>> Michał Mirosław <mirq-linux@rere.qmqm.pl> writes:
>> 
>> > On Fri, Jan 17, 2020 at 08:29:33AM +0300, Sergey Organov wrote:
>> >> --- a/drivers/usb/gadget/function/u_serial.c
>> >> +++ b/drivers/usb/gadget/function/u_serial.c
>> >> @@ -563,6 +563,8 @@ static int gs_start_io(struct gs_port *port)
>> >>  
>> >>         /* unblock any pending writes into our circular buffer */
>> >>         if (started) {
>> >> +               pr_debug("gs_start_tx: ttyGS%d\n", port->port_num);
>> >> +               gs_start_tx(port);
>> >>                 tty_wakeup(port->port.tty);
>> >
>> > The tty_wakeup() will be called from gs_start_tx(), so should be removed
>> > from here.
>> 
>> Not exactly. tty_wakeup() will be called from gs_start_tx() only when
>> there has been something actually transferred from the buffer. I didn't
>> want to change behavior when the buffer is empty, so I kept the explicit
>> tty_wakeup() call in place, intentionally. Please let me know if you
>> still think it should be removed.
>
> Indeed it is as you describe. You might add an argument that initializes
> do_tty_wake, but I'm not sure saving one tty_wakeup() on open is worth
> the trouble.

OK, so let's leave it as is, at least for now.

>
>> > The pr_debug() in other callers of gs_start_tx() say:
>> > "caller: start ttyGS%d".
>> 
>> ???
>> 
>> $ git co gregkh/tty-next && grep -r 'caller: start tty' .
>> HEAD is now at 7788f54... serial_core: Remove unused member in uart_port
>> $ 
>
> Replace 'caller' with a function calling gs_start_io().

Thanks, now I see... Do you prefer:

   pr_debug("gs_start_io: start Tx on ttyGS%d\n", port->port_num);

then?

Alternatively, I'm OK with removing this new debug print.

What do you think?

Thanks,
-- Sergey Organov