Re: [PATCH v1 8/8] phy: qcom-ufs: Refactor all init steps into phy_poweron

Stephen Boyd
2019-01-18T23:50:17Z

Quoting Evan Green (2019-01-11 15:01:29)
> diff --git a/drivers/phy/qualcomm/phy-qcom-ufs.c b/drivers/phy/qualcomm/phy-qcom-ufs.c
> index f2979ccad00a3..a4cff17fef925 100644
> --- a/drivers/phy/qualcomm/phy-qcom-ufs.c
> +++ b/drivers/phy/qualcomm/phy-qcom-ufs.c
> @@ -147,6 +147,21 @@ struct phy *ufs_qcom_phy_generic_probe(struct platform_device *pdev,
> }
> EXPORT_SYMBOL_GPL(ufs_qcom_phy_generic_probe);
>
> +static int ufs_qcom_phy_get_reset(struct ufs_qcom_phy *phy_common)
> +{
> + struct reset_control *reset;
> +
> + if (phy_common->ufs_reset)
> + return 0;
> +
> + reset = of_reset_control_get_by_index(phy_common->dev->of_node, 0);

Same question about using non-DT specific APIs to get the reset control
here.

> + if (IS_ERR(reset))
> + return PTR_ERR(reset);
> +
> + phy_common->ufs_reset = reset;
> + return 0;
> +}
> +
> static int __ufs_qcom_phy_clk_get(struct device *dev,
> const char *name, struct clk **clk_out, bool err_print)
> {
> @@ -528,23 +543,42 @@ int ufs_qcom_phy_power_on(struct phy *generic_phy)
> {
> struct ufs_qcom_phy *phy_common = get_ufs_qcom_phy(generic_phy);
> struct device *dev = phy_common->dev;
> + bool is_rate_B = false;
> int err;
>
> - if (phy_common->is_powered_on)
> - return 0;
> + err = ufs_qcom_phy_get_reset(phy_common);
> + if (err)
> + return err;
>
> - if (!phy_common->is_started) {
> - err = ufs_qcom_phy_start_serdes(phy_common);
> + if (phy_common->ufs_reset) {
> + err = reset_control_assert(phy_common->ufs_reset);
> if (err)
> return err;
> + }
>
> - err = ufs_qcom_phy_is_pcs_ready(phy_common);
> - if (err)
> - return err;
> + if (phy_common->mode == PHY_MODE_UFS_HS_B)
> + is_rate_B = true;
>
> - phy_common->is_started = true;
> + err = phy_common->phy_spec_ops->calibrate(phy_common, is_rate_B);

Is there always a calibrate phys_spec_ops function assigned?

> + if (err)
> + return err;
> +
> + if (phy_common->ufs_reset) {
> + err = reset_control_deassert(phy_common->ufs_reset);
> + if (err) {
> + dev_err(dev, "Failed to assert UFS PHY reset");
> + return err;
> + }
> }