Messages in this thread | | | Subject | Re: [PATCH 1/2] regulator: anatop: Add power gating support to digital LDOs | From | Philipp Zabel <> | Date | Mon, 10 Feb 2014 14:27:55 +0100 |
| |
Hi Mark,
Am Montag, den 10.02.2014, 13:15 +0000 schrieb Mark Brown: > On Thu, Feb 06, 2014 at 03:43:32PM +0100, Philipp Zabel wrote: > > The ARM, PU, and SOC LDOs in the i.MX6 PMU can completely gate > > their power output. Since power gating is configured by writing > > zero to the voltage target bitfield,, store a copy of the > > voltage selector to be restored when reenabling the regulator. > > This is mostly good but... > > > static int anatop_regmap_set_voltage_sel(struct regulator_dev *reg, > > unsigned selector) > > { > > struct anatop_regulator *anatop_reg = rdev_get_drvdata(reg); > > + int ret; > > > > if (!anatop_reg->control_reg) > > return -ENOTSUPP; > > > > - return regulator_set_voltage_sel_regmap(reg, selector); > > + ret = regulator_set_voltage_sel_regmap(reg, selector); > > + if (!ret) > > + anatop_reg->sel = selector; > > + return ret; > > } > > ...I don't understand this. If the regulator is disabled won't this > cause it to be reenabled since we just write the stored selector in to > do that? What I'd expect to see happening is the data being written to > the cache always and only written to the hardware if it's enabled.
yes, thanks. I'll fix this. What should happen if the regulator_set_voltage_sel_regmap fails? Is it ok to still set the cache in this case?
> > +static int anatop_regmap_disable(struct regulator_dev *reg) > > +{ > > + struct anatop_regulator *anatop_reg = rdev_get_drvdata(reg); > > + > > + if (!anatop_is_core_reg(anatop_reg)) > > + return -ENOTSUPP; > > + > > + return regulator_set_voltage_sel_regmap(reg, LDO_POWER_GATE); > > +} > > It's starting to seem like it's worth having separate ops for the core > regulator rather than all these conditionals.
Will do.
regards Philipp
| |