Hi,
On Wed, Sep 5, 2018 at 5:20 PM, Matthias Kaehlcke <m...@chromium.org> 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 <m...@chromium.org>
Thanks for the reviews!
-Doug
