lkml.org 
[lkml]   [2018]   [May]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v3 2/2] regulator: add QCOM RPMh regulator driver
Hi,

On Fri, May 11, 2018 at 7:28 PM, David Collins <collinsd@codeaurora.org> 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

\
 
 \ /
  Last update: 2018-05-17 23:24    [W:0.095 / U:0.052 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site