| Message ID | 20170119104739.4376-2-bjorn.andersson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/4] phy: qcom-ufs: Don't kfree devres resource | expand |
On 2017-01-19 16:17, Bjorn Andersson wrote: > When regulator_get() tries to resolve a regulator supply but fail to > find a matching property in DeviceTree it returns a dummy regulator, if > a matching supply is specified but unavailable the regulator core will > return an error. > > Based on this we should not ignore errors upon failing to acquire the > optional "vddp-ref-clk" supply. > > Cc: Subhash Jadavani <subhashj@codeaurora.org> > Cc: Vivek Gautam <vivek.gautam@codeaurora.org> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- Yea, we don't need additional flag to indicate optional regulator. Change looks good. Reviewed-by: Vivek Gautam <vivek.gautam@codeaurora.org> Regards Vivek > drivers/phy/phy-qcom-ufs.c | 21 +++++++-------------- > 1 file changed, 7 insertions(+), 14 deletions(-) > > diff --git a/drivers/phy/phy-qcom-ufs.c b/drivers/phy/phy-qcom-ufs.c > index 4d7f3c018223..bbd317158084 100644 > --- a/drivers/phy/phy-qcom-ufs.c > +++ b/drivers/phy/phy-qcom-ufs.c > @@ -210,8 +210,9 @@ int ufs_qcom_phy_init_clks(struct ufs_qcom_phy > *phy_common) > } > EXPORT_SYMBOL_GPL(ufs_qcom_phy_init_clks); > > -static int __ufs_qcom_phy_init_vreg(struct device *dev, > - struct ufs_qcom_phy_vreg *vreg, const char *name, bool optional) > +static int ufs_qcom_phy_init_vreg(struct device *dev, > + struct ufs_qcom_phy_vreg *vreg, > + const char *name) > { > int err = 0; > > @@ -221,9 +222,7 @@ static int __ufs_qcom_phy_init_vreg(struct device > *dev, > vreg->reg = devm_regulator_get(dev, name); > if (IS_ERR(vreg->reg)) { > err = PTR_ERR(vreg->reg); > - vreg->reg = NULL; > - if (!optional) > - dev_err(dev, "failed to get %s, %d\n", name, err); > + dev_err(dev, "failed to get %s, %d\n", name, err); > goto out; > } > > @@ -263,12 +262,6 @@ static int __ufs_qcom_phy_init_vreg(struct device > *dev, > return err; > } > > -static int ufs_qcom_phy_init_vreg(struct device *dev, > - struct ufs_qcom_phy_vreg *vreg, const char *name) > -{ > - return __ufs_qcom_phy_init_vreg(dev, vreg, name, false); > -} > - > int ufs_qcom_phy_init_vregulators(struct ufs_qcom_phy *phy_common) > { > int err; > @@ -284,9 +277,9 @@ int ufs_qcom_phy_init_vregulators(struct > ufs_qcom_phy *phy_common) > if (err) > goto out; > > - /* vddp-ref-clk-* properties are optional */ > - __ufs_qcom_phy_init_vreg(phy_common->dev, &phy_common->vddp_ref_clk, > - "vddp-ref-clk", true); > + err = ufs_qcom_phy_init_vreg(phy_common->dev, > &phy_common->vddp_ref_clk, > + "vddp-ref-clk"); > + > out: > return err; > } -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2017-01-19 02:47, Bjorn Andersson wrote: > When regulator_get() tries to resolve a regulator supply but fail to > find a matching property in DeviceTree it returns a dummy regulator, if > a matching supply is specified but unavailable the regulator core will > return an error. > > Based on this we should not ignore errors upon failing to acquire the > optional "vddp-ref-clk" supply. > > Cc: Subhash Jadavani <subhashj@codeaurora.org> > Cc: Vivek Gautam <vivek.gautam@codeaurora.org> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > drivers/phy/phy-qcom-ufs.c | 21 +++++++-------------- > 1 file changed, 7 insertions(+), 14 deletions(-) > > diff --git a/drivers/phy/phy-qcom-ufs.c b/drivers/phy/phy-qcom-ufs.c > index 4d7f3c018223..bbd317158084 100644 > --- a/drivers/phy/phy-qcom-ufs.c > +++ b/drivers/phy/phy-qcom-ufs.c > @@ -210,8 +210,9 @@ int ufs_qcom_phy_init_clks(struct ufs_qcom_phy > *phy_common) > } > EXPORT_SYMBOL_GPL(ufs_qcom_phy_init_clks); > > -static int __ufs_qcom_phy_init_vreg(struct device *dev, > - struct ufs_qcom_phy_vreg *vreg, const char *name, bool optional) > +static int ufs_qcom_phy_init_vreg(struct device *dev, > + struct ufs_qcom_phy_vreg *vreg, > + const char *name) > { > int err = 0; > > @@ -221,9 +222,7 @@ static int __ufs_qcom_phy_init_vreg(struct device > *dev, > vreg->reg = devm_regulator_get(dev, name); > if (IS_ERR(vreg->reg)) { > err = PTR_ERR(vreg->reg); > - vreg->reg = NULL; > - if (!optional) > - dev_err(dev, "failed to get %s, %d\n", name, err); > + dev_err(dev, "failed to get %s, %d\n", name, err); > goto out; > } > > @@ -263,12 +262,6 @@ static int __ufs_qcom_phy_init_vreg(struct device > *dev, > return err; > } > > -static int ufs_qcom_phy_init_vreg(struct device *dev, > - struct ufs_qcom_phy_vreg *vreg, const char *name) > -{ > - return __ufs_qcom_phy_init_vreg(dev, vreg, name, false); > -} > - > int ufs_qcom_phy_init_vregulators(struct ufs_qcom_phy *phy_common) > { > int err; > @@ -284,9 +277,9 @@ int ufs_qcom_phy_init_vregulators(struct > ufs_qcom_phy *phy_common) > if (err) > goto out; > > - /* vddp-ref-clk-* properties are optional */ > - __ufs_qcom_phy_init_vreg(phy_common->dev, &phy_common->vddp_ref_clk, > - "vddp-ref-clk", true); > + err = ufs_qcom_phy_init_vreg(phy_common->dev, > &phy_common->vddp_ref_clk, > + "vddp-ref-clk"); > + > out: > return err; > } Looks good to me. Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org> -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/phy/phy-qcom-ufs.c b/drivers/phy/phy-qcom-ufs.c index 4d7f3c018223..bbd317158084 100644 --- a/drivers/phy/phy-qcom-ufs.c +++ b/drivers/phy/phy-qcom-ufs.c @@ -210,8 +210,9 @@ int ufs_qcom_phy_init_clks(struct ufs_qcom_phy *phy_common) } EXPORT_SYMBOL_GPL(ufs_qcom_phy_init_clks); -static int __ufs_qcom_phy_init_vreg(struct device *dev, - struct ufs_qcom_phy_vreg *vreg, const char *name, bool optional) +static int ufs_qcom_phy_init_vreg(struct device *dev, + struct ufs_qcom_phy_vreg *vreg, + const char *name) { int err = 0; @@ -221,9 +222,7 @@ static int __ufs_qcom_phy_init_vreg(struct device *dev, vreg->reg = devm_regulator_get(dev, name); if (IS_ERR(vreg->reg)) { err = PTR_ERR(vreg->reg); - vreg->reg = NULL; - if (!optional) - dev_err(dev, "failed to get %s, %d\n", name, err); + dev_err(dev, "failed to get %s, %d\n", name, err); goto out; } @@ -263,12 +262,6 @@ static int __ufs_qcom_phy_init_vreg(struct device *dev, return err; } -static int ufs_qcom_phy_init_vreg(struct device *dev, - struct ufs_qcom_phy_vreg *vreg, const char *name) -{ - return __ufs_qcom_phy_init_vreg(dev, vreg, name, false); -} - int ufs_qcom_phy_init_vregulators(struct ufs_qcom_phy *phy_common) { int err; @@ -284,9 +277,9 @@ int ufs_qcom_phy_init_vregulators(struct ufs_qcom_phy *phy_common) if (err) goto out; - /* vddp-ref-clk-* properties are optional */ - __ufs_qcom_phy_init_vreg(phy_common->dev, &phy_common->vddp_ref_clk, - "vddp-ref-clk", true); + err = ufs_qcom_phy_init_vreg(phy_common->dev, &phy_common->vddp_ref_clk, + "vddp-ref-clk"); + out: return err; }
When regulator_get() tries to resolve a regulator supply but fail to find a matching property in DeviceTree it returns a dummy regulator, if a matching supply is specified but unavailable the regulator core will return an error. Based on this we should not ignore errors upon failing to acquire the optional "vddp-ref-clk" supply. Cc: Subhash Jadavani <subhashj@codeaurora.org> Cc: Vivek Gautam <vivek.gautam@codeaurora.org> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> --- drivers/phy/phy-qcom-ufs.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html