Messages in this thread | | | Date | Fri, 22 Apr 2022 14:42:41 +0800 | Subject | Re: [RESEND PATCH -next] USB: serial: pl2303: implement reset_resume member | From | Hongyu Xie <> |
| |
Hi greg On 2022/4/22 13:07, Greg KH wrote: > On Fri, Apr 22, 2022 at 10:35:59AM +0800, Hongyu Xie wrote: >> >> Hi greg, >> On 2022/4/22 00:45, Greg KH wrote: >>> On Tue, Apr 19, 2022 at 02:54:08PM +0800, Hongyu Xie wrote: >>>> From: Hongyu Xie <xiehongyu1@kylinos.cn> >>>> >>>> pl2303.c doesn't have reset_resume for hibernation. >>>> So needs_binding will be set to 1 duiring hibernation. >>>> usb_forced_unbind_intf will be called, and the port minor >>>> will be released (x in ttyUSBx). >>> >>> Please use the full 72 columns that you are allowed in a changelog text. >>> >>> >>>> It works fine if you have only one USB-to-serial device. >>>> Assume you have 2 USB-to-serial device, nameing A and B. >>>> A gets a smaller minor(ttyUSB0), B gets a bigger one. >>>> And start to hibernate. When your PC is in hibernation, >>>> unplug device A. Then wake up your PC by pressing the >>>> power button. After waking up the whole system, device >>>> B gets ttyUSB0. This will casuse a problem if you were >>>> using those to ports(like opened two minicom process) >>>> before hibernation. >>>> So member reset_resume is needed in usb_serial_driver >>>> pl2303_device. >>> >>> If you want persistent device naming, use the symlinks that udev creates >>> for your for all your serial devices. Never rely on the number of a USB >>> to serial device. >> Let me put it this way. Assume you need to record messages output from two >> machines using 2 USB-to-serial devices(naming A and B, and A is on >> USB1-port3, B is on USB1-port4) opened by two minicom process. >> The setting for A in minicom would be like: >> "A - Serial Device : /dev/ttyUSB0" >> The setting for B in minicom would be like: >> "A - Serial Device : /dev/ttyUSB1" >> Then start to hibernate on your computer. When your PC is in >> hibernation, unplug A. After waking up your computer, "/dev/ttyUSB0" >> would be released first, then allocated to B. The minicom process used >> to record outputs from A is now recording B's outputs. The minicom >> process used to record outputs from B is now recording nothing, because >> "/dev/ttyUSB1" is not exist anymore. That's the problem I've been >> talking about. And I don't think using symlinks will solve this problem. > > Yes, symlinks will solve the issue, that is what they are there for. > Look in /dev/serial/ for a persistent name for them that allows you to > uniquely open the correct device if they can be described. Using > /dev/ttyUSBX is almost never the correct thing to do. Thanks for letting me know this. This patch is useless. I still have one more question though, since using /dev/ttyUSBX is almost never the correct thing to do, what is /dev/ttyUSBx used for then? > >>>> Codes in pl2303_reset_resume are borrowed from pl2303_open. >>>> >>>> As a matter of fact, all driver under drivers/usb/serial >>>> has the same problem except ch341.c. >>>> >>>> Cc: stable@vger.kernel.org >>> >>> How does this meet the stable kernel rule requirements? It would be a >>> new feature if it were accepted, right? >> It's not a new feature at all. struct usb_serial_driver already has a >> member name reset_resume, there is no implementation in pl2303.c yet. >> And ch341.c has one(ch341_reset_resume()), that why I said "all driver >> under drivers/usb/serial has the same problem except ch341.c" > > Please read: > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html > for what is valid stable kernel changes. > > thanks, > > greg k-h thanks,
Hongyu Xie
| |