Hi,
On Thu, Jun 13, 2019 at 9:37 AM Marc Gonzalez <[email protected]> wrote: > > On 13/06/2019 18:11, Doug Anderson wrote: > > > On Thu, Jun 13, 2019 at 9:04 AM Marc Gonzalez wrote: > > > >> Hmmm, I expect the typical use-case to be: > >> "HW manual states operation X completes in 100 µs. > >> Let's call usleep_range(100, foo); before hitting the reg." > >> > >> And foo needs to be a "reasonable" value: big enough to be able > >> to merge several requests, low enough not to wait too long after > >> the HW is ready. > >> > >> In this case, I'd say usleep_range(100, 200); makes sense. > >> > >> Come to think of it, I'm not sure min=26 (or min=50) makes sense... > >> Why wait *less* than what the user specified? > > > > IIRC usleep_range() nearly always tries to sleep for the max. My > > recollection of the design is that you only end up with something less > > than the max if the system was going to wake up anyway. In such a > > case it seems like it wouldn't be insane to go and check if the > > condition is already true if 25% of the time has passed. Maybe you'll > > get lucky and you can return early. > > > > Are you actually seeing problems with the / 4, or is this patch just a > > result of code inspection? > > No actual issue. I just ran into a driver calling: > > readl_poll_timeout(status, val, val & mask, 1, 1000); > > and it seemed... unwise(?) to call usleep_range(1, 1); > > But if, as you say, usleep_range() aims for the max It was certainly what we found in: https://lkml.kernel.org/r/[email protected] ...in fact, at one point in time I had a patch cooked up that would change the behavior during boot where (presumably) we'd rather boot faster. ...but after fixing dwc2 it didn't actually have much of an impact elsewhere. > then I guess it's > not a big deal to issue an early read or 3... Meh IMO it seems like the driver should be fixed. It should either specify: a) the (well defined) 0 for the delay, which means no delay. b) a more sane delay -Doug
