RE: [PATCH 2/2] usb: host: ehci-platform: add a quirk to avoid stuck

[Alan Stern <stern@rowland.harvard.edu>]

On Wed, 22 Jan 2020, Yoshihiro Shimoda wrote:

> > Okay, now I understand.  I misread the code in the original patch.
> > But now it looks like the code does roughly this:
> > 
> > Timer routine:	if (ehci_platform_quirk_poll_check_condition(ehci))
> > 			schedule_work();
> > 
> > Work routine:	usleep_range(4000, 8000);
> > 		udelay(10);
> > 		if (!ehci_platform_quirk_poll_check_condition(ehci))
> > 			return;
> > 		udelay(10);
> > 		if (!ehci_platform_quirk_poll_check_condition(ehci))
> > 			return;
> > 		ehci_platform_quirk_poll_rebind_companion(ehci);
> > 
> > So there are three calls to quirk_poll_check_condition, with 4 - 8 ms
> > between the first and second, and 10 us between the second and third.
> > Do you really need to have this combination of a long delay followed by
> > a short delay?  Wouldn't two check_condition calls with only one delay
> > be good enough?
> 
> I had implemented this code by using hardware team's suggestion without
> any doubt. So, I asked hardware team about this combination of delays.
> The hardware team said this combination can reduce misdetection ratio
> from noise and so on. They also said we can wait single 5 ms instead
> this combination (but this cannot reduce misdetection ratio).

Sure, the more times you delay and recheck, the better the error rate.  
But you don't want to go too far.

> So, now I'm thinking that the following process (single wait) is
> enough and it can improve readability. But, what do you think?
> 
> Timer routine:	if (ehci_platform_quirk_poll_check_condition(ehci))
>  			schedule_delayed_work(5 ms);
> 
> Delayed work routine:
> 		if (!ehci_platform_quirk_poll_check_condition(ehci))
>  			return;
>  		ehci_platform_quirk_poll_rebind_companion(ehci);

That looks good to me.

Alan Stern