Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <99546b13-44f1-3dbf-aeb4-86c76ce74626@gmail.com>
Date:   Fri, 22 Apr 2022 14:42:41 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.7.0
Subject: Re: [RESEND PATCH -next] USB: serial: pl2303: implement reset_resume
 member
Content-Language: en-US
To:     Greg KH <gregkh@linuxfoundation.org>
Cc:     johan@kernel.org, linux-usb@vger.kernel.org,
        linux-kernel@vger.kernel.org, Hongyu Xie <xiehongyu1@kylinos.cn>,
        stable@vger.kernel.org, "sheng . huang" <sheng.huang@ecastech.com>,
        wangqi@kylinos.cn, xiongxin@kylinos.cn
References: <20220419065408.2461091-1-xy521521@gmail.com>
 <YmGKL05dnA+q/HAM@kroah.com> <f3f6ea7d-2051-7a7f-61e0-8a5bba8ca8f2@gmail.com>
 <YmI4I9MCLBheMyvr@kroah.com>
From:   Hongyu Xie <xy521521@gmail.com>
In-Reply-To: <YmI4I9MCLBheMyvr@kroah.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


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