Re: [PATCH] USB: serial: use tty_port_register_device_serdev

["Måns Rullgård" <mans@mansr.com>]

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:

> On Thu, May 02, 2024 at 02:24:41PM +0100, Måns Rullgård wrote:
>> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>> 
>> > On Thu, May 02, 2024 at 11:45:44AM +0100, Måns Rullgård wrote:
>> >> Johan Hovold <johan@kernel.org> writes:
>> >> 
>> >> > On Thu, May 02, 2024 at 11:07:28AM +0100, Mans Rullgard wrote:
>> >> >> Use tty_port_register_device_serdev() so that usb-serial devices
>> >> >> can be used as serdev controllers.
>> >> >
>> >> > I'm afraid it's not that easy. The reason serdev is not enabled for
>> >> > usb-serial is that there's currently no support for handling hotplug in
>> >> > serdev. The device can go away from under you at any time and then you'd
>> >> > crash the kernel.
>> >> 
>> >> Oh, that's unfortunate.  Regular serial ports can go away too, though,
>> >> and that seems to be handled fine.  What am I missing?
>> >
>> > How is it handled?  Normal serial ports can go away but in practice,
>> > it's a rare occurance, and usually people use serdev for devices where
>> > the ports can not be removed (i.e. internal connections).
>> 
>> If I unbind a regular serial port from its driver using sysfs, a serdev
>> device defined in a device tree gets removed as expected.  Binding the
>> serial port makes everything come back again.  I fail to see any problem
>> here.  If there is one, you'll have to be less evasive in explaining
>> what it is.
>
> Try yanking a usb-serial device out with this patch applied and see what
> happens.  I'm pretty sure serdev will not handle that well, just like if
> you yank out a pci serial device while it is being used.  Doing
> bind/unbind is not a "surprise" removal, but a nice orderly one :)
>
> If this does now work, nice, but I haven't seen the changes to serdev to
> make this happen, I wonder what changed...

Turns out I missed one change that is needed for unplugging to be
handled:

--- a/drivers/usb/serial/bus.c
+++ b/drivers/usb/serial/bus.c
@@ -91,7 +91,7 @@ static void usb_serial_device_remove(struct device *dev)
        autopm_err = usb_autopm_get_interface(port->serial->interface);
 
        minor = port->minor;
-       tty_unregister_device(usb_serial_tty_driver, minor);
+       tty_port_unregister_device(&port->port, usb_serial_tty_driver, minor);
 
        driver = port->serial->type;
        if (driver->port_remove)

With this additional change, yanking (shorting the data lines; the thing
is soldered) the usb-serial converter works, although a couple of
warnings are printed:

[   28.678301] usb 1-1: USB disconnect, device number 2
[   28.683695] ttyUSB ttyUSB0: tty_hangup: tty->count(1) != (#fd's(0) + #kopen's(0))
[   28.691516] ftdi_sio ttyUSB0: error from flowcontrol urb
[   28.759056] ttyUSB ttyUSB0: tty_port_close_start: tty->count = 1 port count = 0
[   28.772531] ftdi_sio ttyUSB0: FTDI USB Serial Device converter now disconnected from ttyUSB0
[   28.781346] ftdi_sio 1-1:1.0: device disconnected

Where should I go looking for the cause of these?

-- 
Måns Rullgård