Hi,
On Fri, May 11, 2018 at 7:28 PM, David Collins <collinsd@xxxxxxxxxxxxxx> wrote:
> +static int rpmh_regulator_parse_vrm_modes(struct rpmh_vreg *vreg,
> + struct device *dev, struct device_node *node)
> +{
> + const char *prop;
> + int i, len, ret, mode;
> + u32 *buf;
> +
> + /* qcom,allowed-drms-modes is optional */
> + prop = "qcom,allowed-drms-modes";
As per comments in bindings patch: this is a duplicate of your new
attribute you added to the regulator core. Makes no sense to have a
private attribute too with the same value.
> + prop = "qcom,drms-mode-max-microamps";
As per comments in the bindings patch, I think we should move
"qcom,drms-mode-max-microamps" to the regulator core.
> + prop = "qcom,regulator-initial-microvolt";
As per comments in bindings patch: seems like we should get rid of
"qcom,regulator-initial-microvolt" or move to the core.
> + /*
> + * Default the voltage selector to an error value in the
> + * case that qcom,regulator-initial-microvolt is not
> + * specified in device tree since the true voltage is
> + * not known. Note that this value causes
> + * devm_regulator_register() to fail in the case that
> + * regulator-min-microvolt and regulator-max-microvolt
> + * are specified in device tree due to
> + * machine_constraints_voltage() bailing when the
> + * get_voltage_sel() callback returns this error value.
> + */
> + vreg->voltage_selector = -EINVAL;
As per comments in other threads, adjust this comment and use
-ENOTRECOVERABLE now.
NOTE: I think this driver is looking really good now. Hopefully the
above things should be quick to spin (even getting "max-microamps" in
the core should be quick I think) and we can get something landed!
:)
-Doug
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
