Re: [PATCH 1/2] spi: Add MXIC controller driver

[Boris Brezillon <boris.brezillon@xxxxxxxxxxx>]

On Fri, 3 Aug 2018 13:06:46 +0200
Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:

> Hi Boris,
> 
> On Fri, Aug 3, 2018 at 9:48 AM Boris Brezillon
> <boris.brezillon@xxxxxxxxxxx> wrote:
> > On Thu, 2 Aug 2018 19:27:45 +0000
> > Trent Piepho <tpiepho@xxxxxxxxxx> wrote:  
> > > On Thu, 2018-08-02 at 10:07 +0800, masonccyang@xxxxxxxxxxx wrote:  
> > > > +   ret = clk_set_phase(mxic->send_dly_clk, 360 / (1000000000 / freq));  
> > >
> > > Can also be written as freq*9/25000000.  
> >
> > I'd like to keep 360 here. The phase is in degree, hence the 360. The
> > compiler should be smart enough to optimize that at compilation
> > time ;-).  
> 
> It may have a hard time optimizing that, while preserving the semantics
> of the original expression (rounding behavior!)...
> 
> Speaking about that, there's a serious issue due to loss of accuracy when
> doing two divisions like that. "360 * freq / 1000000000" is better, assumed
> the multiplication will be done in 64-bit arithmetic to avoid overflow:
> 
>     u64 t = 360ULL * freq;
>     ret = clk_set_phase(mxic->send_dly_clk, do_div(t, 1000000000));

True.

> 
> BTW, can freq < 1000000000 / 360?

Theoretically yes, in practice, that's unlikely.

Anyway, I still don't quite understand where this formula comes from,
so I'll let Mason details why the phase has to be set to this value...
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html