Hi, Mark On Thursday, June 09, 2016 01:16 AM, Mark Brown wrote: > On Mon, May 09, 2016 at 03:05:08PM +0800, WEN Pingbo wrote: > >> And regulator core will postpone all operations until all consumers >> have taked their place. > > It doesn't, it postpones them until late_initacall(). This is both > after the consumers have loaded if they are built in and before any > consumers built as modules come up.
Yes, this patch only protects a regulator from regulator registration(built in) to late_initcall(). But, IMO, if a regulator is critical, it's weird to build as a module. Maybe I was thoughtless here. If we take modules under consideration, and to make this patch more universal, I think what we really need is adding a flag to protect a regulator from registration to a specific consumer(not the first consumer). The regulator driver gives the initial state, and the specific consumer need to clear this flag while finishing regulator setting(by calling a function like regulator_clear_protect()). And what the regulator core need to do is staging all operations during protection. And that will cover all consumers probing order, whenever the regulator is registered. Any idea? > >> The boot_protection flag only work before late_initicall. And as other >> constraints liked, you can specify this flag in a board file, or in >> dts file. > > Anything added to the DT ABI needs a binding. > I will add bindings in next version. >> + /* constraints check has already done */ >> + if (rdev->boot_mode) >> + rdev->desc->ops->set_mode(rdev, rdev->boot_mode); > > This whole sequence of code ignores errors - that's not great. We > should at least log them. > OK. >> + mutex_unlock(&rdev->mutex); >> + >> + if (regulator) >> + regulator_set_voltage(regulator, regulator->min_uV, >> + regulator->max_uV); > > That's... exciting. There's a couple of issues here. One is that > this is not operating on the rdev but rather on a consumer regulator > device, the other is that we drop out of the lock before doing the > update which tends to be a warning sign that something fun is going on > and at least an internal function should be used. These two most likely > come down to the same issue. > OK, some bugs here. I will use a unlock version. Pingbo
