RE: [PATCH] i2c: riic: remove clock and frequency restrictions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Geert,

On Wednesday, October 18, 2017 1, Geert Uytterhoeven wrote:
> >> > +       /*
> >> > +        * Remove clock ticks for rise and fall times. Convert ns to
> >> clock
> >> > +        * ticks.
> >> > +        */
> >> > +       brl -= t->scl_fall_ns / (1000000000/rate);
> >> > +       brh -= t->scl_rise_ns / (1000000000/rate);
> >>
> >> To avoid losing too much precision by the division, you can rewrite
> this
> >> as
> >> e.g.
> >>
> >>     brl -= t->scl_fall_ns * rate / 1000000000;
> >>
> >> ("scl_fall_ns * rate" should not overflow).
> >> Perhaps DIV_ROUND_UP(), too?
> >
> > Unfortunately, I do not get the correct number every time.
> >
> > Sometimes I get the correct number, but most of the time I get 0.
> >
> > Note, if rate=33,330,000 and t->scl_fall_ns=300, then
> >
> > t->scl_fall_ns * rate  = 9,999,000,000 (0x253FCA1C0) a 34-bit number.
> >
> > So, I'd rather just stick with the less precision because it's not that
> > much of a bit deal.
> 
> Oops, so the numbers are bigger than I had expected, naively.
> 
> Alternatively, you can use a 64-by-32 division, e.g.
> 
>     DIV_ROUND_UP_ULL((u64)t->scl_fall_ns * rate, 1000000000)

That works better.

However, rounding up on this calculation means you may run faster than 
Specified (ie, you spend more time HIGH or LOW)


As a raw math vs code comparison:

t->scl_fall_ns = 300
rate = 16662500    (33325000/2)
REAL number = 4.99875
DIV_ROUND_UP_ULL produces = 5

t->scl_fall_ns = 250
rate = 16662500    (33325000/2)
REAL number = 4.165625
DIV_ROUND_UP_ULL produces = 5 (now I'm running higher faster than 400kHz)


So on this number, rounding down (truncating) is 'safer' from a speed 
stand point.

What do you think?

Chris





[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux