Messages in this thread | | | Date | Fri, 24 Mar 2017 13:38:19 -0700 | From | Brian Norris <> | Subject | Re: [PATCH] regulator: core: Limit propagation of parent voltage count and list |
| |
On Fri, Mar 24, 2017 at 01:09:52PM -0700, Matthias Kaehlcke wrote: > diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c > index 53d4fc70dbd0..121838e0125b 100644 > --- a/drivers/regulator/core.c > +++ b/drivers/regulator/core.c > @@ -2487,6 +2487,10 @@ static int _regulator_list_voltage(struct regulator *regulator, > if (lock) > mutex_unlock(&rdev->mutex); > } else if (rdev->supply) { > + // Limit propagation of parent values to switch regulators
The kernel doesn't use C99 comments. Oddly enough, this isn't actually in the coding style doc (Documentation/process/coding-style.rst), nor is it caught by scripts/checkpatch.pl (even though it clearly has a 'C99 comment' rule).
> + if (ops->get_voltage || ops->get_voltage_sel) > + return -EINVAL; > + > ret = _regulator_list_voltage(rdev->supply, selector, lock); > } else { > return -EINVAL; > @@ -2540,6 +2544,7 @@ EXPORT_SYMBOL_GPL(regulator_is_enabled); > int regulator_count_voltages(struct regulator *regulator) > { > struct regulator_dev *rdev = regulator->rdev; > + const struct regulator_ops *ops = rdev->desc->ops; > > if (rdev->desc->n_voltages) > return rdev->desc->n_voltages; > @@ -2547,6 +2552,10 @@ int regulator_count_voltages(struct regulator *regulator) > if (!rdev->supply) > return -EINVAL; > > + // Limit propagation of parent value to switch regulators
Same here.
> + if (ops->get_voltage || ops->get_voltage_sel || ops->list_voltage) > + return -EINVAL; > + > return regulator_count_voltages(rdev->supply); > } > EXPORT_SYMBOL_GPL(regulator_count_voltages);
I'm not very familiar with this code, but judging by your problem description in previous threads and by comparing with the logic in _regulator_get_voltage() (for when to reference the ->supply), this seems resonable. So:
Reviewed-by: Brian Norris <briannorris@chromium.org>
It's probably worth verifying that this doesn't break whatever Javier was supporting in the first place, as a sanity check.
Brian
| |