On Fri, 2019-10-25 at 18:51 +0300, Andriy Gapon wrote: > On 25/10/2019 18:46, Ian Lepore wrote: > > On Fri, 2019-10-25 at 15:38 +0000, Andriy Gapon wrote: > > > Author: avg > > > Date: Fri Oct 25 15:38:09 2019 > > > New Revision: 354076 > > > URL: https://svnweb.freebsd.org/changeset/base/354076 > > > > > > Log: > > > owc_gpiobus_read_data: compare times in sbintime_t units > > > > > > Previously the code used sbttous() before microseconds > > > comparison > > > in one > > > place, sbttons() and nanoseconds in another, division by > > > SBT_1US > > > and > > > microseconds in yet another. > > > > > > Now the code consistently uses multiplication by SBT_1US to > > > convert > > > microseconds to sbintime_t before comparing them with periods > > > between > > > calls to sbinuptime(). This is fast, this is precise enough > > > (below > > > 0.03%) and the periods defined by the protocol cannot overflow. > > > > > > Reviewed by: imp (D22108) > > > MFC after: 2 weeks > > > > > > Modified: > > > head/sys/dev/ow/owc_gpiobus.c > > > > > > Modified: head/sys/dev/ow/owc_gpiobus.c > > > ================================================================= > > > ==== > > > ========= > > > --- head/sys/dev/ow/owc_gpiobus.c Fri Oct 25 15:02:50 2019 ( > > > r354 > > > 075) > > > +++ head/sys/dev/ow/owc_gpiobus.c Fri Oct 25 15:38:09 2019 ( > > > r354 > > > 076) > > > @@ -296,10 +296,10 @@ owc_gpiobus_read_data(device_t dev, struct > > > ow_timing * > > > do { > > > now = sbinuptime(); > > > GETPIN(sc, &sample); > > > - } while (sbttous(now - then) < t->t_rdv + 2 && sample == 0); > > > + } while (now - then < (t->t_rdv + 2) * SBT_1US && sample == 0); > > > critical_exit(); > > > > > > - if (sbttons(now - then) < t->t_rdv * 1000) > > > + if (now - then < t->t_rdv * SBT_1US) > > > *bit = 1; > > > else > > > *bit = 0; > > > @@ -307,7 +307,7 @@ owc_gpiobus_read_data(device_t dev, struct > > > ow_timing * > > > /* Wait out the rest of t_slot */ > > > do { > > > now = sbinuptime(); > > > - } while ((now - then) / SBT_1US < t->t_slot); > > > + } while (now - then < t->t_slot * SBT_1US); > > > > > > RELBUS(sc); > > > > > > > Unit conversions with sbt times should be done using the macros > > that > > carefully avoid roundoff errors. I don't understand why you've > > changed > > the code that correctly did use those macros to inline math. > > I think that the commit message explains it: > This is fast, this is precise enough (below 0.03%) and the periods > defined by > the protocol cannot overflow. > > Do you disagree? > Could you please explain in which of the three lines changed the new > code is > worse and why? >
I absolutely disagree (or I wouldn't have replied). Unit conversions using sbt times should use the predefined macros, NOT incline multiplication and division. I don't know how to say it more clearly than that. The conversion macros are idiomatic (at least, they would be if people stopped writing inline conversion expressions). -- Ian _______________________________________________ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
