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
