Date: Fri, 25 Oct 2019 18:51:16 +0300 From: Andriy Gapon <avg@FreeBSD.org> To: Ian Lepore <ian@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r354076 - head/sys/dev/ow Message-ID: <2fde5b7b-6186-0969-08a5-83524b6aa274@FreeBSD.org> In-Reply-To: <ca3e991449dda0ed70bbc578aa6069cefce40fc4.camel@freebsd.org> References: <201910251538.x9PFc9ii028313@repo.freebsd.org> <ca3e991449dda0ed70bbc578aa6069cefce40fc4.camel@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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? -- Andriy Gapon
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?2fde5b7b-6186-0969-08a5-83524b6aa274>