lkml.org 
[lkml]   [2012]   [Sep]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/8] sta2x11-mfd : add apb-soc regs driver and factor out common code
    On Wed, Sep 26, 2012 at 05:49:33PM +0100, Mark Brown wrote:
    > On Wed, Sep 26, 2012 at 06:31:45PM +0200, Davide Ciminaghi wrote:
    >
    > > Oh, and there's another problem (I'm looking at the code right now, I had
    > > forgotten about this): the clock framework also asks for a spinlock_t *.
    > > Regmap has its own spinlock (or mutex), and I don't think it would be a
    > > good idea trying to export it.
    >
    > Why is this a problem? Nested spinlocks are perfectly fine.

    after some cups of coffee, I tried thinking about this subject again, and
    here's what I found:

    as far as I know, nested locks are fine provided that you always take them in
    the same order and release them in the opposite order (lock A, lock B,
    unlock B, unlock A). So my conclusion is that nested spinlocks require
    potential regmap users of sta2x11 registers to take the sta2x11-mfd spinlock
    first. The pattern would be (sctl registers for instance):

    spinlock_t *mfd_lock;
    void *addr;

    /*
    Read sta2x11-mfd spinlock address for sctl registers and put it into
    mfd_lock
    */
    sta2x11_mfd_get_regs_data(pdev, sta2x11_sctl, &addr, &mfd_lock);

    ......

    /* Take the outer lock */
    spin_lock(mfd_lock);
    /*
    Take the inner lock, do whatever you need to do and release the inner lock
    */
    regmap_...();
    /* Release the outer lock */
    spin_unlock(mfd_lock);

    The other way around (taking the regmap spinlock as the "outer" one)
    would imply at least exporting the regmap spinlock/mutex address or making the
    regmap.lock/unlock function pointers public somehow, not
    to mention the fact that, for instance, the clock framework helpers should be
    modified to deal with nested locking.

    If the above is correct, I'm not sure we'd gain a lot from regmap,
    because we'd still need some sta2x11 extra code around regmap calls.

    Maybe there's another solution: what about adding a couple of function
    pointers (lock, unlock) to struct regmap_config ? If they're set to NULL,
    everything works as usual. If they're not NULL, regmap uses such functions
    to do locking and unlocking instead of the usual ones (and regmap_bus.fast_io
    is ignored). I think this wouldn't hurt existing regmap users and allow
    sta2x11-mfd to have just one spinlock for everything, no need for nested
    locking this way.

    Thanks and regards
    Davide


    \
     
     \ /
      Last update: 2012-09-27 16:21    [W:4.378 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site