lkml.org 
[lkml]   [2015]   [Mar]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 03/12] clocksource: Remove clocksource_max_deferment()

    * John Stultz <john.stultz@linaro.org> wrote:

    > clocksource_max_deferment() doesn't do anything useful
    > anymore, so zap it.

    Well, it does something useful, it encapsulates the max_deferment
    property of the clocksource:

    > -static u64 clocksource_max_deferment(struct clocksource *cs)
    > -{
    > - u64 max_nsecs;
    > -
    > - max_nsecs = clocks_calc_max_nsecs(cs->mult, cs->shift, cs->maxadj,
    > - cs->mask);
    > - return max_nsecs;
    > -}

    Which could be written in a shorter form, using:

    static u64 clocksource_max_deferment(struct clocksource *cs)
    {
    return clocks_calc_max_nsecs(cs->mult, cs->shift, cs->maxadj, cs->mask);
    }

    Which all allows short forms of:

    cs->max_idle_ns = clocksource_max_deferment(cs);

    without writing out all the arguments.

    Instead of that, you've introduced:

    > + cs->max_idle_ns = clocks_calc_max_nsecs(cs->mult, cs->shift,
    > + cs->maxadj, cs->mask);

    > + cs->max_idle_ns = clocks_calc_max_nsecs(cs->mult, cs->shift,
    > + cs->maxadj, cs->mask);

    Which in the next patch gets even worse:


    > cs->max_idle_ns = clocks_calc_max_nsecs(cs->mult, cs->shift,
    > + cs->maxadj, cs->mask,
    > + &cs->max_cycles);

    > /* calculate max idle time permitted for this clocksource */
    > cs->max_idle_ns = clocks_calc_max_nsecs(cs->mult, cs->shift,
    > + cs->maxadj, cs->mask,
    > + &cs->max_cycles);

    While with the helper function it would still be the same sweet:

    cs->max_idle_ns = clocksource_max_deferment(cs);

    So I don't think this cleanup is an improvement ...

    Thanks,

    Ingo


    \
     
     \ /
      Last update: 2015-03-07 10:41    [W:2.402 / U:0.596 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site