Re: [PATCH 2/2] soc: qcom: geni: geni_se_clk_freq_match() should always accept multiples

[Doug Anderson]

Hi,

On Wed, Sep 5, 2018 at 5:20 PM, Matthias Kaehlcke <mka@xxxxxxxxxxxx> wrote:
> On Thu, Aug 30, 2018 at 11:36:12AM -0700, Douglas Anderson wrote:
>> The geni_se_clk_freq_match() has some strange semantics.  Specifically
>> it is defined with two modes:
>> 1. It can find a clock that's an exact multiple of the requested rate
>> 2. If can find a non-exact match but it can't handle multiples then
>
> nit: s/If/It/

Splleing was nvevr my strong suit.  Done.


>> +     best_delta = 0;
>>       for (i = 0; i < num_clk_levels; i++) {
>> -             if (!(tbl[i] % req_freq)) {
>> +             divider = DIV_ROUND_UP(tbl[i], req_freq);
>> +             new_delta = req_freq - tbl[i] / divider;
>> +             if (!best_delta || new_delta < best_delta) {
>
> nit: if you assigned best_delta to ULONG_MAX above you could omit the
> check for !best_delta here, better to read IMO.

Done.


> Looks good to me assuming that the statement "callers should always be
> able to handle a clock that is a multiple of the requested clock" is
> correct.

I think we should be OK given that currently there's no real callers.
If we really need to add another parameter to disallow multiples we
can always do it later.


> Reviewed-by: Matthias Kaehlcke <mka@xxxxxxxxxxxx>

Thanks for the reviews!

-Doug