Hi Subhash,


On Wed, Sep 27, 2017 at 4:43 AM, Subhash Jadavani
[off-list ref] wrote:

Hi Vivek,

Please find one comment inline below, rest look good.

Regards,
Subhash


On 2017-08-03 23:48, Vivek Gautam wrote:

quoted

Refactor ufs_qcom_power_up_sequence() to get rid of ugly
exported phy APIs and use the phy_init() and phy_power_on()
to do the phy initialization.

Signed-off-by: Vivek Gautam <redacted>
---
 drivers/phy/qualcomm/phy-qcom-ufs-i.h        |  2 --
 drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c |  9 +++++--
 drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c |  9 +++++--
 drivers/phy/qualcomm/phy-qcom-ufs.c          | 38
++++++++--------------------
 drivers/scsi/ufs/ufs-qcom.c                  | 36
++++++++++----------------
 include/linux/phy/phy-qcom-ufs.h             |  3 ---
 6 files changed, 38 insertions(+), 59 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-ufs-i.h

b/drivers/phy/qualcomm/phy-qcom-ufs-i.h
index 94326ed107c3..495fd5941231 100644

--- a/drivers/phy/qualcomm/phy-qcom-ufs-i.h
+++ b/drivers/phy/qualcomm/phy-qcom-ufs-i.h

@@ -123,7 +123,6 @@ struct ufs_qcom_phy {
  * struct ufs_qcom_phy_specific_ops - set of pointers to functions which

have a
  * specific implementation per phy. Each UFS phy, should implement
  * those functions according to its spec and requirements
- * @calibrate_phy: pointer to a function that calibrate the phy
  * @start_serdes: pointer to a function that starts the serdes
  * @is_physical_coding_sublayer_ready: pointer to a function that
  * checks pcs readiness. returns 0 for success and non-zero for error.

@@ -132,7 +131,6 @@ struct ufs_qcom_phy {
  * and writes to QSERDES_RX_SIGDET_CNTRL attribute
  */
 struct ufs_qcom_phy_specific_ops {
-       int (*calibrate_phy)(struct ufs_qcom_phy *phy, bool is_rate_B);
        void (*start_serdes)(struct ufs_qcom_phy *phy);
        int (*is_physical_coding_sublayer_ready)(struct ufs_qcom_phy

*phy);
        void (*set_tx_lane_enable)(struct ufs_qcom_phy *phy, u32 val);

diff --git a/drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c

b/drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c
index af65785230b5..c39440b56b6d 100644

--- a/drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c
+++ b/drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c

@@ -44,7 +44,13 @@ void ufs_qcom_phy_qmp_14nm_advertise_quirks(struct

ufs_qcom_phy *phy_common)

 static int ufs_qcom_phy_qmp_14nm_init(struct phy *generic_phy)
 {
-       return 0;
+       struct ufs_qcom_phy *phy_common = get_ufs_qcom_phy(generic_phy);
+       bool is_rate_B = false;
+
+       if (phy_common->mode == PHY_MODE_UFS_HS_B)
+               is_rate_B = true;
+
+       return ufs_qcom_phy_qmp_14nm_phy_calibrate(phy_common, is_rate_B);
 }

 static int ufs_qcom_phy_qmp_14nm_exit(struct phy *generic_phy)

@@ -120,7 +126,6 @@ static int

ufs_qcom_phy_qmp_14nm_is_pcs_ready(struct ufs_qcom_phy *phy_common)
 };

 static struct ufs_qcom_phy_specific_ops phy_14nm_ops = {
-       .calibrate_phy          = ufs_qcom_phy_qmp_14nm_phy_calibrate,
        .start_serdes           = ufs_qcom_phy_qmp_14nm_start_serdes,
        .is_physical_coding_sublayer_ready =
ufs_qcom_phy_qmp_14nm_is_pcs_ready,
        .set_tx_lane_enable     =
ufs_qcom_phy_qmp_14nm_set_tx_lane_enable,

diff --git a/drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c

b/drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c
index 5c18c41dbdb4..5705a2d4c6d2 100644

--- a/drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c
+++ b/drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c

@@ -63,7 +63,13 @@ void ufs_qcom_phy_qmp_20nm_advertise_quirks(struct

ufs_qcom_phy *phy_common)

 static int ufs_qcom_phy_qmp_20nm_init(struct phy *generic_phy)
 {
-       return 0;
+       struct ufs_qcom_phy *phy_common = get_ufs_qcom_phy(generic_phy);
+       bool is_rate_B = false;
+
+       if (phy_common->mode == PHY_MODE_UFS_HS_B)
+               is_rate_B = true;
+
+       return ufs_qcom_phy_qmp_20nm_phy_calibrate(phy_common, is_rate_B);
 }

 static int ufs_qcom_phy_qmp_20nm_exit(struct phy *generic_phy)

@@ -178,7 +184,6 @@ static int

ufs_qcom_phy_qmp_20nm_is_pcs_ready(struct ufs_qcom_phy *phy_common)
 };

 static struct ufs_qcom_phy_specific_ops phy_20nm_ops = {
-       .calibrate_phy          = ufs_qcom_phy_qmp_20nm_phy_calibrate,
        .start_serdes           = ufs_qcom_phy_qmp_20nm_start_serdes,
        .is_physical_coding_sublayer_ready =
ufs_qcom_phy_qmp_20nm_is_pcs_ready,
        .set_tx_lane_enable     =
ufs_qcom_phy_qmp_20nm_set_tx_lane_enable,

diff --git a/drivers/phy/qualcomm/phy-qcom-ufs.c

b/drivers/phy/qualcomm/phy-qcom-ufs.c
index 43865ef340e2..1febe3294fe3 100644

--- a/drivers/phy/qualcomm/phy-qcom-ufs.c
+++ b/drivers/phy/qualcomm/phy-qcom-ufs.c

@@ -518,9 +518,8 @@ void ufs_qcom_phy_disable_iface_clk(struct

ufs_qcom_phy *phy)
        }
 }

-int ufs_qcom_phy_start_serdes(struct phy *generic_phy)
+static int ufs_qcom_phy_start_serdes(struct ufs_qcom_phy *ufs_qcom_phy)
 {
-       struct ufs_qcom_phy *ufs_qcom_phy = get_ufs_qcom_phy(generic_phy);
        int ret = 0;

        if (!ufs_qcom_phy->phy_spec_ops->start_serdes) {

@@ -533,7 +532,6 @@ int ufs_qcom_phy_start_serdes(struct phy *generic_phy)

        return ret;
 }
-EXPORT_SYMBOL_GPL(ufs_qcom_phy_start_serdes);

 int ufs_qcom_phy_set_tx_lane_enable(struct phy *generic_phy, u32

tx_lanes)
 {

@@ -564,31 +562,8 @@ void ufs_qcom_phy_save_controller_version(struct

phy *generic_phy,
 }
 EXPORT_SYMBOL_GPL(ufs_qcom_phy_save_controller_version);

-int ufs_qcom_phy_calibrate_phy(struct phy *generic_phy, bool is_rate_B)
-{
-       struct ufs_qcom_phy *ufs_qcom_phy = get_ufs_qcom_phy(generic_phy);
-       int ret = 0;
-
-       if (!ufs_qcom_phy->phy_spec_ops->calibrate_phy) {
-               dev_err(ufs_qcom_phy->dev, "%s: calibrate_phy() callback
is not supported\n",
-                       __func__);
-               ret = -ENOTSUPP;
-       } else {
-               ret = ufs_qcom_phy->phy_spec_ops->
-                               calibrate_phy(ufs_qcom_phy, is_rate_B);
-               if (ret)
-                       dev_err(ufs_qcom_phy->dev, "%s: calibrate_phy()
failed %d\n",
-                               __func__, ret);
-       }
-
-       return ret;
-}
-EXPORT_SYMBOL_GPL(ufs_qcom_phy_calibrate_phy);
-
-int ufs_qcom_phy_is_pcs_ready(struct phy *generic_phy)
+static int ufs_qcom_phy_is_pcs_ready(struct ufs_qcom_phy *ufs_qcom_phy)
 {
-       struct ufs_qcom_phy *ufs_qcom_phy = get_ufs_qcom_phy(generic_phy);
-
        if
(!ufs_qcom_phy->phy_spec_ops->is_physical_coding_sublayer_ready) {
                dev_err(ufs_qcom_phy->dev, "%s:
is_physical_coding_sublayer_ready()
callback is not supported\n",
                        __func__);

@@ -598,7 +573,6 @@ int ufs_qcom_phy_is_pcs_ready(struct phy *generic_phy)
        return ufs_qcom_phy->phy_spec_ops->
                        is_physical_coding_sublayer_ready(ufs_qcom_phy);
 }
-EXPORT_SYMBOL_GPL(ufs_qcom_phy_is_pcs_ready);

 int ufs_qcom_phy_power_on(struct phy *generic_phy)
 {

@@ -609,6 +583,14 @@ int ufs_qcom_phy_power_on(struct phy *generic_phy)
        if (phy_common->is_powered_on)
                return 0;

+       err = ufs_qcom_phy_start_serdes(phy_common);
+       if (err)
+               return err;
+
+       err = ufs_qcom_phy_is_pcs_ready(phy_common);
+       if (err)
+               return err;
+

Now that we are doing serdes start (and checking the PCS ready), I am not
sure we can call phy_power_on() from ufs_qcom_resume(). Please check.

Right. Thanks for catching this. I will check this further.

BRs
Vivek

[snip]


-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation