Re: [PATCH 1/3] clk: ti: add 'ti,round-rate' flag

[Mike Turquette <mturquette@linaro.org>]

Quoting Tomi Valkeinen (2014-05-15 05:25:37)
> On 15/05/14 09:08, Mike Turquette wrote:
> > Quoting Tomi Valkeinen (2014-05-12 05:13:51)
> >> On 12/05/14 15:02, Tero Kristo wrote:
> >>> On 05/08/2014 12:06 PM, Tomi Valkeinen wrote:
> >>>> The current DPLL code does not try to round the clock rate, and instead
> >>>> returns an error if the requested clock rate cannot be produced exactly
> >>>> by the DPLL.
> >>>>
> >>>> It could be argued that this is a bug, but as the current drivers may
> >>>> depend on that behavior, a new flag 'ti,round-rate' is added which
> >>>> enables clock rate rounding.
> >>>
> >>> Someone could probably argue that this flag is not a hardware feature,
> >>
> >> I fully agree.
> >>
> >>> but instead is used to describe linux-kernel behavior, and would
> >>> probably be frowned upon by the DT enthusiasts. Othen than that, I like
> >>> this approach better than a global setting, but would like second
> >>> opinions here.
> >>
> >> I think the dpll code should always do rounding. That's what
> >> round_rate() is supposed to do, afaik. The current behavior of not
> >> rounding and returning an error is a bug in my opinion.
> > 
> > From include/linux/clk.h:
> > 
> > /**
> >  * clk_round_rate - adjust a rate to the exact rate a clock can provide
> >  * @clk: clock source
> >  * @rate: desired clock rate in Hz
> >  *
> >  * Returns rounded clock rate in Hz, or negative errno.
> >  */
> > long clk_round_rate(struct clk *clk, unsigned long rate);
> > 
> > Definitely not rounding the rate is a bug, with respect to the API
> > definition. Has anyone tried making the new flag as the default behavior
> > and seeing if anything breaks?
> 
> The v1 of the patch fixed the rounding unconditionally:
> 
> http://article.gmane.org/gmane.linux.ports.arm.kernel/295077
> 
> Paul wanted it optional so that existing drivers would not break. No one
> knows if there is such a driver, or what would the driver's code look
> like that would cause an issue.
> 
> And, as I've pointed out in the above thread, as clk-divider driver
> doesn't an error code from the dpll driver, my opinion is that such
> drivers would not work even now.
> 
> I like v1 more.
> 
> In any case, I hope we'd get something merged ASAP so that we fix the
> display AM3xxx boards and we'd still have time to possibly find out if
> some other driver breaks.

Resurrecting this thread. Can we reach a consensus? I prefer V1 as well
for the reasons stated above, and also since ti,round-rate isn't really
describing the hardware at all in DT.

We can always see how it goes and fix it up afterwards when everything
explodes.

Regards,
Mike

> 
>  Tomi
> 
>