Messages in this thread | | | Date | Tue, 3 May 2011 22:41:55 +0100 | From | Mark Brown <> | Subject | Re: [RFC][PATCH] mfd: mc13xxx-core: put mutex lock down to mc13xxx_reg_rmw function |
| |
On Tue, May 03, 2011 at 09:15:25PM +0200, Uwe Kleine-König wrote: > On Wed, May 04, 2011 at 12:27:59AM +0800, Axel Lin wrote:
> > + ret = mc13xxx_reg_rmw(priv->mc13xxx, mc13892_regulators[id].vsel_reg, > > + mask, value << mc13892_regulators[id].vsel_shift);
> I havn't looked deeply, but I guess this can have unwanted side effects > here. Before you had:
> lock() > do(something) > do(something, else, that, needs, rmw) > unlock()
> and you introduced an unlock()/lock() between these two do()s.
Glancing at the code I wasn't 100% convinced that the original read was really needed, though I didn't look closely.
> I'm not convinced this change is good, though I agree that
> lock() > rmw(...) > unlock()
> looks ugly, but imho this can better be fixed by adding a wrapper for > that sequence if you really want.
You could also make the rmw store the value somewhere if it's important. Having to open code the locks everywhere is certainly annoying and error prone. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |