lkml.org 
[lkml]   [2011]   [Jan]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/2] Add a common struct clk
    On Wed, Jan 05, 2011 at 11:51:02AM +0800, Jeremy Kerr wrote:

    > + * The @lock member provides either a spinlock or a mutex to protect (at least)
    > + * @enable_count. The type of lock used will depend on @flags; if CLK_ATOMIC is
    > + * set, then the core clock code will use a spinlock, otherwise a mutex. This
    > + * lock will be acquired during clk_enable and clk_disable, so for atomic
    > + * clocks, these ops callbacks must not sleep.
    > + *
    > + * The choice of atomic or non-atomic clock depends on how the clock is enabled.
    > + * Typically, you'll want to use a non-atomic clock. For clocks that need to be
    > + * enabled/disabled in interrupt context, use CLK_ATOMIC. Note that atomic
    > + * clocks with parents will typically cascade enable/disable operations to
    > + * their parent, so the parent of an atomic clock *must* be atomic too.

    ...

    > +struct clk {
    > + const struct clk_ops *ops;
    > + unsigned int enable_count;
    > + int flags;
    > + union {
    > + struct mutex mutex;
    > + spinlock_t spinlock;
    > + } lock;
    > +};

    Here you have a "polymorphic" lock, where the clock instance knows
    which type it is supposed to be. I got flak from David Miller and
    others trying to do the same thing with the mdio_bus:

    http://kerneltrap.org/mailarchive/linux-netdev/2010/7/6/6280618

    The criticism, applied to your case, is that the clk_enable() caller
    cannot know whether it is safe to make the call or not. I was told,
    "there has got to be a better way."

    Richard



    \
     
     \ /
      Last update: 2011-01-06 17:11    [W:0.025 / U:30.128 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site