Hi, Mark > 在 2016年4月25日,06:51,Mark Brown <broo...@kernel.org> 写道: > > On Sat, Apr 23, 2016 at 03:11:06PM +0800, WEN Pingbo wrote: > >> This patch try to add a boot_protection flag in regulator constraints. >> So the regulator core will prevent the specified operation during kernel >> booting. > >> 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. By default, all operations of this regulator will be rejected >> during kernel booting, if you add this flag in a regulator. But you >> still have a chance to change this, by modifying boot_valid_ops_mask. > > This is still a complete hack which is going to break as soon as things > are built modular, it's definitely *not* something that should ever > appear in DT since it depends so heavily on implementation details. If > you need some driver to start early work on getting that sorted. >
I think this patch can handle the case you mentioned. I have add a regulator_has_booted flag, and it will set in regulator_init_complete() late_initcall hook. The regulator_ops_is_valid() will ignore boot protection if this flag is set. > This is also going to interact badly with any other drivers that are > trying to configure things at runtime, if they've done enables and > disables (or especially an enable without a matching disable) their > refcounts are going to be wrong and if they've tried to do anything with > setting voltages we'll have completely ignored whatever they asked for > or told them that they can't change voltages. If we were doing anything > like this it would need to be a lot more transparent to other > regulators sharing the supplies (which are presumably what's causing > problems here). Ok, I have to admit that the boot_protection didn’t cover this. If other consumer try to configure during booting, it will get some error code. And the consumers need to re-configure the regulator state after late_initcall. If we need to hold the state of other consumer, I prefer using a dummy-consumer to hold this. And this is my next try. > >> @@ -868,7 +877,7 @@ static void print_constraints(struct regulator_dev *rdev) >> rdev_dbg(rdev, "%s\n", buf); >> >> if ((constraints->min_uV != constraints->max_uV) && >> - !regulator_ops_is_valid(rdev, REGULATOR_CHANGE_VOLTAGE)) >> + !(constraints->valid_ops_mask & REGULATOR_CHANGE_VOLTAGE)) >> rdev_warn(rdev, >> "Voltage range but no REGULATOR_CHANGE_VOLTAGE\n"); >> } > > This appears to be unrelated? > >> + if (constraints->boot_protection) { >> + if (of_property_read_bool(np, "boot-allow-set-voltage")) >> + constraints->boot_valid_ops_mask |= >> + REGULATOR_CHANGE_VOLTAGE; > > We were factoring things out a minute ago… Actually, the two part are only want to check the regulator operation capacity, if we include the boot_protect checking, it will get error. Pingbo