Messages in this thread | | | Date | Thu, 27 Sep 2012 15:41:10 +0200 | From | Davide Ciminaghi <> | Subject | Re: [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
| |