lkml.org 
[lkml]   [2014]   [Jun]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC 4/5] clk: per-user clock accounting for debug
    On 06/27/2014 01:57 AM, Tomeu Vizoso wrote:
    > From: Rabin Vincent <rabin.vincent@stericsson.com>
    >
    > When a clock has multiple users, the WARNING on imbalance of
    > enable/disable may not show the guilty party since although they may
    > have commited the error earlier, the warning is emitted later when some
    > other user, presumably innocent, disables the clock.
    >
    > Provide per-user clock enable/disable accounting and disabler tracking
    > in order to help debug these problems.
    >
    > NOTE: with this patch, clk_get_parent() behaves like clk_get(), i.e. it
    > needs to be matched with a clk_put(). Otherwise, memory will leak.

    > diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h
    > index 91659b2..9657fc8 100644
    > --- a/include/linux/clk-private.h
    > +++ b/include/linux/clk-private.h
    > @@ -56,7 +56,11 @@ struct clk_core {
    > };
    >
    > struct clk {
    > - struct clk_core clk;
    > + struct clk_core *core;
    > + unsigned int enable_count;
    > + const char *dev_id;
    > + const char *con_id;

    Why not just store the "struct device *" there instead of pulling the
    name out of it, so ...

    > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c

    > void clk_disable(struct clk *clk_user)
    > {
    > - __clk_disable_internal(clk_to_clk_core(clk_user));
    > + struct clk_core *clk = clk_to_clk_core(clk_user);
    > + unsigned long flags;
    > +
    > + flags = clk_enable_lock();
    > + if (!WARN(clk_user->enable_count == 0,
    > + "incorrect disable clk dev %s con %s last disabler %pF\n",
    > + clk_user->dev_id, clk_user->con_id, clk_user->last_disable)) {

    Here, you could do something like:

    if (!clk_user->enable_count) {
    dev_err(clk_user->dev, "", ...);
    goto out;
    }
    ...
    out:
    clk_enable_unlock(flags);
    }

    I suppose that has the disadvantage of not using WARN() so not
    generating a full back-trace. Still, you could keep the use of WARN()
    and pass as a parameter to the printf parameters dev_name(clk_user->dev)
    rather than manually saving the fields separately.


    \
     
     \ /
      Last update: 2014-06-28 01:01    [W:2.660 / U:0.040 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site