| Message ID | 20230425034010.3789376-3-quic_bjorande@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | phy: qcom-qmp-combo: Support orientation switching | expand |
On Mon, Apr 24, 2023 at 08:40:05PM -0700, Bjorn Andersson wrote: > With the upcoming introduction of USB Type-C orientation switching the > region of mutual exclusion needs to be extended to cover both the common > init/exit as well as the individual functions. > > So move the phy_mutex one step up the stack. > > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> > --- > drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 51 +++++++++++++---------- > 1 file changed, 30 insertions(+), 21 deletions(-) > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > index 6850e04c329b..7280f7141961 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > @@ -2463,16 +2463,13 @@ static int qmp_combo_com_init(struct qmp_combo *qmp) > void __iomem *com = qmp->com; > int ret; > > - mutex_lock(&qmp->phy_mutex); > - if (qmp->init_count++) { > - mutex_unlock(&qmp->phy_mutex); > + if (qmp->init_count++) > return 0; > - } > > ret = regulator_bulk_enable(cfg->num_vregs, qmp->vregs); > if (ret) { > dev_err(qmp->dev, "failed to enable regulators, err=%d\n", ret); > - goto err_unlock; > + goto err; I was going to say that you can just return ret directly but then realised we have a counter imbalance here that should be fixed. I've just sent a couple of fixes which you could rebase on: https://lore.kernel.org/r/20230502103810.12061-1-johan+linaro@kernel.org > } > > ret = reset_control_bulk_assert(cfg->num_resets, qmp->resets); > @@ -2514,16 +2511,13 @@ static int qmp_combo_com_init(struct qmp_combo *qmp) > qphy_setbits(qmp->pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL], > SW_PWRDN); > > - mutex_unlock(&qmp->phy_mutex); > - > return 0; > > err_assert_reset: > reset_control_bulk_assert(cfg->num_resets, qmp->resets); > err_disable_regulators: > regulator_bulk_disable(cfg->num_vregs, qmp->vregs); > -err_unlock: > - mutex_unlock(&qmp->phy_mutex); > +err: > > return ret; > } > @@ -2686,14 +2683,19 @@ static int qmp_combo_usb_init(struct phy *phy) > struct qmp_combo *qmp = phy_get_drvdata(phy); > int ret; > > + mutex_lock(&qmp->phy_mutex); Nit: I think adding a newline here would improve readability. > ret = qmp_combo_com_init(qmp); > if (ret) > - return ret; > + goto out_unlock; > > ret = qmp_combo_usb_power_on(phy); > - if (ret) > + if (ret) { > qmp_combo_com_exit(qmp); > + goto out_unlock; > + } > > +out_unlock: > + mutex_unlock(&qmp->phy_mutex); Same here. > return ret; > } > > @@ -2702,11 +2704,18 @@ static int qmp_combo_usb_exit(struct phy *phy) > struct qmp_combo *qmp = phy_get_drvdata(phy); > int ret; > > + mutex_lock(&qmp->phy_mutex); And here. > ret = qmp_combo_usb_power_off(phy); > if (ret) > - return ret; > + goto out_unlock; > > - return qmp_combo_com_exit(qmp); > + ret = qmp_combo_com_exit(qmp); > + if (ret) > + goto out_unlock; > + > +out_unlock: > + mutex_unlock(&qmp->phy_mutex); And here. > + return ret; > } > > static int qmp_combo_usb_set_mode(struct phy *phy, enum phy_mode mode, int submode) Looks good otherwise: Reviewed-by: Johan Hovold <johan+linaro@kernel.org> Johan
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c index 6850e04c329b..7280f7141961 100644 --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c @@ -2463,16 +2463,13 @@ static int qmp_combo_com_init(struct qmp_combo *qmp) void __iomem *com = qmp->com; int ret; - mutex_lock(&qmp->phy_mutex); - if (qmp->init_count++) { - mutex_unlock(&qmp->phy_mutex); + if (qmp->init_count++) return 0; - } ret = regulator_bulk_enable(cfg->num_vregs, qmp->vregs); if (ret) { dev_err(qmp->dev, "failed to enable regulators, err=%d\n", ret); - goto err_unlock; + goto err; } ret = reset_control_bulk_assert(cfg->num_resets, qmp->resets); @@ -2514,16 +2511,13 @@ static int qmp_combo_com_init(struct qmp_combo *qmp) qphy_setbits(qmp->pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL], SW_PWRDN); - mutex_unlock(&qmp->phy_mutex); - return 0; err_assert_reset: reset_control_bulk_assert(cfg->num_resets, qmp->resets); err_disable_regulators: regulator_bulk_disable(cfg->num_vregs, qmp->vregs); -err_unlock: - mutex_unlock(&qmp->phy_mutex); +err: return ret; } @@ -2532,11 +2526,8 @@ static int qmp_combo_com_exit(struct qmp_combo *qmp) { const struct qmp_phy_cfg *cfg = qmp->cfg; - mutex_lock(&qmp->phy_mutex); - if (--qmp->init_count) { - mutex_unlock(&qmp->phy_mutex); + if (--qmp->init_count) return 0; - } reset_control_bulk_assert(cfg->num_resets, qmp->resets); @@ -2544,8 +2535,6 @@ static int qmp_combo_com_exit(struct qmp_combo *qmp) regulator_bulk_disable(cfg->num_vregs, qmp->vregs); - mutex_unlock(&qmp->phy_mutex); - return 0; } @@ -2555,21 +2544,29 @@ static int qmp_combo_dp_init(struct phy *phy) const struct qmp_phy_cfg *cfg = qmp->cfg; int ret; + mutex_lock(&qmp->phy_mutex); + ret = qmp_combo_com_init(qmp); if (ret) - return ret; + goto out_unlock; cfg->dp_aux_init(qmp); - return 0; +out_unlock: + mutex_unlock(&qmp->phy_mutex); + return ret; } static int qmp_combo_dp_exit(struct phy *phy) { struct qmp_combo *qmp = phy_get_drvdata(phy); + mutex_lock(&qmp->phy_mutex); + qmp_combo_com_exit(qmp); + mutex_unlock(&qmp->phy_mutex); + return 0; } @@ -2686,14 +2683,19 @@ static int qmp_combo_usb_init(struct phy *phy) struct qmp_combo *qmp = phy_get_drvdata(phy); int ret; + mutex_lock(&qmp->phy_mutex); ret = qmp_combo_com_init(qmp); if (ret) - return ret; + goto out_unlock; ret = qmp_combo_usb_power_on(phy); - if (ret) + if (ret) { qmp_combo_com_exit(qmp); + goto out_unlock; + } +out_unlock: + mutex_unlock(&qmp->phy_mutex); return ret; } @@ -2702,11 +2704,18 @@ static int qmp_combo_usb_exit(struct phy *phy) struct qmp_combo *qmp = phy_get_drvdata(phy); int ret; + mutex_lock(&qmp->phy_mutex); ret = qmp_combo_usb_power_off(phy); if (ret) - return ret; + goto out_unlock; - return qmp_combo_com_exit(qmp); + ret = qmp_combo_com_exit(qmp); + if (ret) + goto out_unlock; + +out_unlock: + mutex_unlock(&qmp->phy_mutex); + return ret; } static int qmp_combo_usb_set_mode(struct phy *phy, enum phy_mode mode, int submode)
With the upcoming introduction of USB Type-C orientation switching the region of mutual exclusion needs to be extended to cover both the common init/exit as well as the individual functions. So move the phy_mutex one step up the stack. Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> --- drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 51 +++++++++++++---------- 1 file changed, 30 insertions(+), 21 deletions(-)