On Mon, May 09, 2016 at 03:05:08PM +0800, WEN Pingbo wrote: > In some platforms, critical shared regulator is initialized in > bootloader. But during kernel booting, the driver probing order and > conflicting operations from other regulator consumers, may set the > regulator in a undefined state, which will cause serious problem.
> This patch try to add a boot_protection flag in regulator constraints. This still feels like a short term hack that doesn't belong in an ABI. It's all very implementation specific and not very robust, it's not describing the outcome we're looking for but rather a very specific behaviour which won't work outside of a fairly narrow system configuration. The difficulty in coherently explaining what the end of boot is and what protection means is a big warning sign here. I think you need to be looking at some combination of getting the devices you're interested in started up early and more precisely describing the end result you're trying to achieve. The issues with probe deferral do seem related here, it's another symptom of not really making any decisions about init ordering and so sometimes making bad ones. > 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. > 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. > + /* 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. > + 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.
signature.asc
Description: PGP signature
