lkml.org 
[lkml]   [2011]   [Feb]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [RFC,PATCH 1/3] Add a common struct clk
    Date
    Hi Saravana,

    > Sure, one could argue that in some archs for a certain set of clocks the
    > slow stuff in prepare/unprepare won't need to be done during set rate --
    > say, a simple clock that always runs off the same PLL but just has a
    > integer divider to change the rate.
    >
    > In those cases, not grabbing the prepare_lock would make the code less
    > "locky".
    >
    > > We
    > > may even want to disallow set_rate (and set_parent) when prepare_count is
    > > non- zero.
    >
    > This is definitely not right.

    Why is that? Consider two devices using one clock; one does some
    initialisation based on the return value of clk_get_rate(), the other calls
    clk_set_rate() some time later. Now the first device is incorrectly
    initialised.

    Regardless, this is definitely something to flag for a later discussion. I'm
    happy to return to that, but we should focus on one issue at a time here.

    > Changing the rate of a clock when it's
    > already enabled/prepared is a very reasonable thing to do. It's only
    > doing a set rate at the "same time" as a prepare/unprepare that's wrong
    > for some clocks. We could have the specific implementation deal with the
    > locking internally.

    Yes, hence leaving the locking here to the clock implementation.

    > > I'd prefer to enforce the 'sleepability' with might_sleep instead.
    >
    > Yeah, I realized this option after sending out my previous email. Please
    > do add a might_sleep(). It will actually point out errors (per the new
    > clarification) in some serial drivers.

    Yep, will do.

    > >>> + .enable_lock = __SPIN_LOCK_UNLOCKED(name.enable_lock), \
    > >>> + .prepare_lock = __MUTEX_INITIALIZER(name.prepare_lock), \
    > >>
    > >> After a long day, I'm not able to wrap my head around this. Probably a
    > >> stupid question, but will this name.xxx thing prevent using this
    > >> INIT_CLK macro to initialize an array of clocks? More specifically,
    > >> prevent the sub class macro (like INIT_CLK_FIXED) from being used to
    > >> initialize an array of clocks?
    > >
    > > That's correct. For an array of clocks, you'll have to use a different
    > > initialiser. We can add helpers for that that when (and if) the need
    > > arises.
    >
    > Would it even be possible to get this to work for an array? You don't
    > have to change this in the patch, but I'm curious to know how to get
    > this to work for an array without doing a run time init of the lock.

    I'd assume that you'd have to do this at run time, as with any other array of
    structs that contain a mutex or spinlock.

    Cheers,


    Jeremy


    \
     
     \ /
      Last update: 2011-02-15 08:29    [W:4.879 / U:0.128 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site