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

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 2274 bytes --]

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.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]