Messages in this thread | | | Subject | Re: [PATCH v1 7/8] phy: qcom-qmp: Move UFS phy to phy_poweron/off | From | Stephen Boyd <> | Date | Fri, 18 Jan 2019 14:39:01 -0800 |
| |
Quoting Evan Green (2019-01-11 15:01:28) > For UFS, move the actual firing up of the PHY to phy_poweron and > phy_poweroff callbacks, rather than init/exit. UFS calls > phy_poweroff during suspend, so now all clocks and regulators for > the phy can be powered down during suspend. > > Signed-off-by: Evan Green <evgreen@chromium.org> > > --- > > drivers/phy/qualcomm/phy-qcom-qmp.c | 82 ++++++++--------------------- > 1 file changed, 23 insertions(+), 59 deletions(-) > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c > index eb1cac8f0fd4e..7766c6384d0a8 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qmp.c > +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c > @@ -1224,7 +1223,6 @@ static int qcom_qmp_phy_init(struct phy *phy) > int ret; > > dev_vdbg(qmp->dev, "Initializing QMP phy\n"); > - > if (cfg->has_ufsphy_reset) { > /* > * Get UFS reset, which is delayed until now to avoid a
Nitpick: Drop this hunk.
> @@ -1360,7 +1353,6 @@ static int qcom_qmp_phy_exit(struct phy *phy) > > /* Put PHY into POWER DOWN state: active low */ > qphy_clrbits(qphy->pcs, QPHY_POWER_DOWN_CONTROL, cfg->pwrdn_ctrl); > - > if (cfg->has_lane_rst) > reset_control_assert(qphy->lane_rst); >
And this hunk.
> @@ -1371,44 +1363,6 @@ static int qcom_qmp_phy_exit(struct phy *phy) > return 0; > } > > -static int qcom_qmp_phy_poweron(struct phy *phy) > -{ > - struct qmp_phy *qphy = phy_get_drvdata(phy); > - struct qcom_qmp *qmp = qphy->qmp; > - const struct qmp_phy_cfg *cfg = qmp->cfg; > - void __iomem *pcs = qphy->pcs; > - void __iomem *status; > - unsigned int mask, val; > - int ret = 0; > - > - if (cfg->type != PHY_TYPE_UFS) > - return 0; > - > - /* > - * For UFS PHY that has not software reset control, serdes start > - * should only happen when UFS driver explicitly calls phy_power_on > - * after it deasserts software reset. > - */ > - if (cfg->no_pcs_sw_reset && !qmp->phy_initialized && > - (qmp->init_count != 0)) { > - /* start SerDes and Phy-Coding-Sublayer */ > - qphy_setbits(pcs, cfg->regs[QPHY_START_CTRL], cfg->start_ctrl); > - > - status = pcs + cfg->regs[QPHY_PCS_READY_STATUS]; > - mask = cfg->mask_pcs_ready; > - > - ret = readl_poll_timeout(status, val, !(val & mask), 1, > - PHY_INIT_COMPLETE_TIMEOUT);
So we never need to poll this bit anymore?
> - if (ret) { > - dev_err(qmp->dev, "phy initialization timed-out\n"); > - return ret; > - } > - qmp->phy_initialized = true; > - } > - > - return ret; > -} > - > static int qcom_qmp_phy_set_mode(struct phy *phy, > enum phy_mode mode, int submode) > { > @@ -1658,9 +1612,15 @@ static int phy_pipe_clk_register(struct qcom_qmp *qmp, struct device_node *np) > } > > static const struct phy_ops qcom_qmp_phy_gen_ops = { > - .init = qcom_qmp_phy_init, > - .exit = qcom_qmp_phy_exit, > - .power_on = qcom_qmp_phy_poweron, > + .init = qcom_qmp_phy_enable, > + .exit = qcom_qmp_phy_disable, > + .set_mode = qcom_qmp_phy_set_mode, > + .owner = THIS_MODULE, > +}; > + > +static const struct phy_ops qcom_qmp_ufs_ops = { > + .power_on = qcom_qmp_phy_enable, > + .power_off = qcom_qmp_phy_disable, > .set_mode = qcom_qmp_phy_set_mode, > .owner = THIS_MODULE, > };
So the UFS and the non-UFS phys will use the same single function, but the callers of the phys will see that phy_power_on() powers on the phy for UFS but does nothing for non-UFS devices? Do the users of this common phy call the API differently between drivers? Kishon, is there guidance on how phys are supposed to be used by drivers?
| |