| Message ID | 20200408012854.3070187-1-bjorn.andersson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | phy: qualcomm: usb-hs-28nm: Prepare clocks in init | expand |
On 08/04/2020 02:28, Bjorn Andersson wrote: > The AHB clock must be on for qcom_snps_hsphy_init() to be able to write > the initialization sequence to the hardware, so move the clock > enablement to phy init and exit. > > Fixes: 67b27dbeac4d ("phy: qualcomm: Add Synopsys 28nm Hi-Speed USB PHY driver") > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > drivers/phy/qualcomm/phy-qcom-usb-hs-28nm.c | 32 ++++++++++++++------- > 1 file changed, 21 insertions(+), 11 deletions(-) > > diff --git a/drivers/phy/qualcomm/phy-qcom-usb-hs-28nm.c b/drivers/phy/qualcomm/phy-qcom-usb-hs-28nm.c > index d998e65c89c8..a52a9bf13b75 100644 > --- a/drivers/phy/qualcomm/phy-qcom-usb-hs-28nm.c > +++ b/drivers/phy/qualcomm/phy-qcom-usb-hs-28nm.c > @@ -160,18 +160,11 @@ static int qcom_snps_hsphy_power_on(struct phy *phy) > ret = regulator_bulk_enable(VREG_NUM, priv->vregs); > if (ret) > return ret; > - ret = clk_bulk_prepare_enable(priv->num_clks, priv->clks); > - if (ret) > - goto err_disable_regulator; > + > qcom_snps_hsphy_disable_hv_interrupts(priv); > qcom_snps_hsphy_exit_retention(priv); > > return 0; > - > -err_disable_regulator: > - regulator_bulk_disable(VREG_NUM, priv->vregs); > - > - return ret; > } > > static int qcom_snps_hsphy_power_off(struct phy *phy) > @@ -180,7 +173,6 @@ static int qcom_snps_hsphy_power_off(struct phy *phy) > > qcom_snps_hsphy_enter_retention(priv); > qcom_snps_hsphy_enable_hv_interrupts(priv); > - clk_bulk_disable_unprepare(priv->num_clks, priv->clks); > regulator_bulk_disable(VREG_NUM, priv->vregs); > > return 0; > @@ -266,21 +258,39 @@ static int qcom_snps_hsphy_init(struct phy *phy) > struct hsphy_priv *priv = phy_get_drvdata(phy); > int ret; > > - ret = qcom_snps_hsphy_reset(priv); > + ret = clk_bulk_prepare_enable(priv->num_clks, priv->clks); > if (ret) > return ret; > > + ret = qcom_snps_hsphy_reset(priv); > + if (ret) > + goto disable_clocks; > + > qcom_snps_hsphy_init_sequence(priv); > > ret = qcom_snps_hsphy_por_reset(priv); > if (ret) > - return ret; > + goto disable_clocks; > + > + return 0; > + > +disable_clocks: > + clk_bulk_disable_unprepare(priv->num_clks, priv->clks); > + return ret; > +} > + > +static int qcom_snps_hsphy_exit(struct phy *phy) > +{ > + struct hsphy_priv *priv = phy_get_drvdata(phy); > + > + clk_bulk_disable_unprepare(priv->num_clks, priv->clks); > > return 0; > } > > static const struct phy_ops qcom_snps_hsphy_ops = { > .init = qcom_snps_hsphy_init, > + .exit = qcom_snps_hsphy_exit, > .power_on = qcom_snps_hsphy_power_on, > .power_off = qcom_snps_hsphy_power_off, > .set_mode = qcom_snps_hsphy_set_mode, > Makes sense to me Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
On Tue 07 Apr 18:28 PDT 2020, Bjorn Andersson wrote: > The AHB clock must be on for qcom_snps_hsphy_init() to be able to write > the initialization sequence to the hardware, so move the clock > enablement to phy init and exit. > > Fixes: 67b27dbeac4d ("phy: qualcomm: Add Synopsys 28nm Hi-Speed USB PHY driver") > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> Kishon, can we have this picked up? Thanks, Bjorn > --- > drivers/phy/qualcomm/phy-qcom-usb-hs-28nm.c | 32 ++++++++++++++------- > 1 file changed, 21 insertions(+), 11 deletions(-) > > diff --git a/drivers/phy/qualcomm/phy-qcom-usb-hs-28nm.c b/drivers/phy/qualcomm/phy-qcom-usb-hs-28nm.c > index d998e65c89c8..a52a9bf13b75 100644 > --- a/drivers/phy/qualcomm/phy-qcom-usb-hs-28nm.c > +++ b/drivers/phy/qualcomm/phy-qcom-usb-hs-28nm.c > @@ -160,18 +160,11 @@ static int qcom_snps_hsphy_power_on(struct phy *phy) > ret = regulator_bulk_enable(VREG_NUM, priv->vregs); > if (ret) > return ret; > - ret = clk_bulk_prepare_enable(priv->num_clks, priv->clks); > - if (ret) > - goto err_disable_regulator; > + > qcom_snps_hsphy_disable_hv_interrupts(priv); > qcom_snps_hsphy_exit_retention(priv); > > return 0; > - > -err_disable_regulator: > - regulator_bulk_disable(VREG_NUM, priv->vregs); > - > - return ret; > } > > static int qcom_snps_hsphy_power_off(struct phy *phy) > @@ -180,7 +173,6 @@ static int qcom_snps_hsphy_power_off(struct phy *phy) > > qcom_snps_hsphy_enter_retention(priv); > qcom_snps_hsphy_enable_hv_interrupts(priv); > - clk_bulk_disable_unprepare(priv->num_clks, priv->clks); > regulator_bulk_disable(VREG_NUM, priv->vregs); > > return 0; > @@ -266,21 +258,39 @@ static int qcom_snps_hsphy_init(struct phy *phy) > struct hsphy_priv *priv = phy_get_drvdata(phy); > int ret; > > - ret = qcom_snps_hsphy_reset(priv); > + ret = clk_bulk_prepare_enable(priv->num_clks, priv->clks); > if (ret) > return ret; > > + ret = qcom_snps_hsphy_reset(priv); > + if (ret) > + goto disable_clocks; > + > qcom_snps_hsphy_init_sequence(priv); > > ret = qcom_snps_hsphy_por_reset(priv); > if (ret) > - return ret; > + goto disable_clocks; > + > + return 0; > + > +disable_clocks: > + clk_bulk_disable_unprepare(priv->num_clks, priv->clks); > + return ret; > +} > + > +static int qcom_snps_hsphy_exit(struct phy *phy) > +{ > + struct hsphy_priv *priv = phy_get_drvdata(phy); > + > + clk_bulk_disable_unprepare(priv->num_clks, priv->clks); > > return 0; > } > > static const struct phy_ops qcom_snps_hsphy_ops = { > .init = qcom_snps_hsphy_init, > + .exit = qcom_snps_hsphy_exit, > .power_on = qcom_snps_hsphy_power_on, > .power_off = qcom_snps_hsphy_power_off, > .set_mode = qcom_snps_hsphy_set_mode, > -- > 2.24.0 >
On 07-04-20, 18:28, Bjorn Andersson wrote: > The AHB clock must be on for qcom_snps_hsphy_init() to be able to write > the initialization sequence to the hardware, so move the clock > enablement to phy init and exit. Applied, thanks
diff --git a/drivers/phy/qualcomm/phy-qcom-usb-hs-28nm.c b/drivers/phy/qualcomm/phy-qcom-usb-hs-28nm.c index d998e65c89c8..a52a9bf13b75 100644 --- a/drivers/phy/qualcomm/phy-qcom-usb-hs-28nm.c +++ b/drivers/phy/qualcomm/phy-qcom-usb-hs-28nm.c @@ -160,18 +160,11 @@ static int qcom_snps_hsphy_power_on(struct phy *phy) ret = regulator_bulk_enable(VREG_NUM, priv->vregs); if (ret) return ret; - ret = clk_bulk_prepare_enable(priv->num_clks, priv->clks); - if (ret) - goto err_disable_regulator; + qcom_snps_hsphy_disable_hv_interrupts(priv); qcom_snps_hsphy_exit_retention(priv); return 0; - -err_disable_regulator: - regulator_bulk_disable(VREG_NUM, priv->vregs); - - return ret; } static int qcom_snps_hsphy_power_off(struct phy *phy) @@ -180,7 +173,6 @@ static int qcom_snps_hsphy_power_off(struct phy *phy) qcom_snps_hsphy_enter_retention(priv); qcom_snps_hsphy_enable_hv_interrupts(priv); - clk_bulk_disable_unprepare(priv->num_clks, priv->clks); regulator_bulk_disable(VREG_NUM, priv->vregs); return 0; @@ -266,21 +258,39 @@ static int qcom_snps_hsphy_init(struct phy *phy) struct hsphy_priv *priv = phy_get_drvdata(phy); int ret; - ret = qcom_snps_hsphy_reset(priv); + ret = clk_bulk_prepare_enable(priv->num_clks, priv->clks); if (ret) return ret; + ret = qcom_snps_hsphy_reset(priv); + if (ret) + goto disable_clocks; + qcom_snps_hsphy_init_sequence(priv); ret = qcom_snps_hsphy_por_reset(priv); if (ret) - return ret; + goto disable_clocks; + + return 0; + +disable_clocks: + clk_bulk_disable_unprepare(priv->num_clks, priv->clks); + return ret; +} + +static int qcom_snps_hsphy_exit(struct phy *phy) +{ + struct hsphy_priv *priv = phy_get_drvdata(phy); + + clk_bulk_disable_unprepare(priv->num_clks, priv->clks); return 0; } static const struct phy_ops qcom_snps_hsphy_ops = { .init = qcom_snps_hsphy_init, + .exit = qcom_snps_hsphy_exit, .power_on = qcom_snps_hsphy_power_on, .power_off = qcom_snps_hsphy_power_off, .set_mode = qcom_snps_hsphy_set_mode,
The AHB clock must be on for qcom_snps_hsphy_init() to be able to write the initialization sequence to the hardware, so move the clock enablement to phy init and exit. Fixes: 67b27dbeac4d ("phy: qualcomm: Add Synopsys 28nm Hi-Speed USB PHY driver") Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> --- drivers/phy/qualcomm/phy-qcom-usb-hs-28nm.c | 32 ++++++++++++++------- 1 file changed, 21 insertions(+), 11 deletions(-)