[lkml]   [2011]   [Jan]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: Locking in the clk API
    Hi Saravana,

    On Tue, Jan 11, 2011 at 07:25:01PM -0800, Saravana Kannan wrote:
    > On 01/11/2011 06:35 AM, Jeremy Kerr wrote:
    > >Hi Paul,
    > >
    > >>Again, you are approaching it from the angle that an atomic clock is a
    > >>special requirement rather than the default behaviour.
    > >
    > >I'm not considering it a special requirement, but it's still a requirement
    > >(that the called function does not sleep).
    > >
    > >The problem with the inverse logic (clk_enable/clk_enable_sleepable) is that
    > >now you've made the caller need to know what kind of clock it has, or might
    > >have one day.
    > I think it's just a matter of how you interpret the name of the API
    > in English. It doesn't make the decision making of the developer any
    > easier.
    > Just having a _atomic suffix doesn't mean the driver developer
    > doesn't need to know what type of clock it is. They are still making
    > the assumption that the enable/disable for that clock can be done
    > atomically -- namely an "atomic clock".
    But there is a difference to 'one function to rule both sleepable and
    atomic clocks'. When calling _atomic on a sleepable clock you get
    -ESOMETHING back (and the clock stays off). With a generic clk_enable
    you get an oops and so cannot handle the error.

    > Similarly, when a driver developer calls the _sleepable APIs in
    > their code, for all practical purposes, they are making an
    > assumption that the enable/disable for that clock *needs to* (not
    > may) sleep.
    IMHO this is not right. If the driver developer doesn't care if the
    clock sleeps or not (which is the norm I think) he calls the _sleepable
    function and if the clock happen to be an atomic one it doesn't hurt

    And looking at the usage of the sleeping functions in the gpio API, I'd
    bet that at least 50% of the calls to gpio_set_value can/should be
    gpio_set_value_cansleep. That's because driver developers don't care or
    are not aware of the issue. If it would be gpio_set_value
    vs.gpio_set_value_atomic most developers would use the sleeping variant
    and the few that should use the _atomic function would notice that when
    seeing the corresponding oops.

    Best regards

    Pengutronix e.K. | Uwe Kleine-König |
    Industrial Linux Solutions | |
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2011-01-12 08:43    [W:0.433 / U:185.224 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site