Re: [PATCH next] usb: usbfs: Add reset_resume for usbfs

[Oliver Neukum <oneukum@suse.com>]

On 17.07.24 03:43, Hongyu Xie wrote:
> From: Hongyu Xie <xiehongyu1@kylinos.cn>

Hi,

sorry for being incomprehensible. I'll try to do better.

>  From what I know, that CONFIG_USB_DEFAULT_PERSIST is enabled by default. Then udev->persist_enabled is set to 1 and this causing udev->reset_resume set to 1 during init2 in hub_activate.
> During resume,
> usb_resume_both
>    usb_resume_device
>      generic_resume
>        usb_port_resume
>          finish_port_resume
>            usb_reset_and_verify_device (if udev->reset_resume is 1)
>              hub_port_init
>                hub_port_reset
>    usb_resume_interface (udev->reset_resume is 1 but usbfs doesn't have reset_resume implementation so set intf->needs_binding to 1, and it will be rebind in usb_resume_complete)

That is correct. But even if CONFIG_USB_DEFAULT_PERSIST were not set, losing power
would just lead to reenumeration by another code path. Devices reset themselves
when they are power cycled. There is no way around that.

> Even before usbfs->reset_resume is called (if there is one), the USB device has already been reset

Yes, it has been reset.

> and in a good state.

No, it is not. Or rather, it is in the wrong state. This is not a question of good and bad.
It is a question of being in the same state.
  
> After all that thaw_processes is called and user-space application runs again.

Yes. And user space does not know what has happened.
> 
> So I still don't understand why "the race necessarily exists". Can you show me an example that usbfs->reset_resume causes race?

Sure. Let's look at the example of a scanner attached to the host.

OS			Scanning process (in user space)
			1. Set a resolution
2. Going to S4
3. Returning to S0
			4. Initiate a scan

As you can see the system would now scan at the wrong resolution. Step#4
has to fail. As there is no synchronization between S4 and user space, initiating
the scan can always be the last step.
For this to work, reset_resume() would have to restore the correct resolution. The kernel
cannot do so.

	Regards
		Oliver