[PATCH 0/5] qcom-ufs: phy/hcd: Refactor phy initialization code

[PATCH 0/5] qcom-ufs: phy/hcd: Refactor phy initialization code

[Vivek Gautam]

Refactoring the qcom-ufs phy and host controller code to move
further towards the generic phy usage. Right now the qcom-ufs exports
a bunch of APIs that are used by the host controller to initialize
the phy.
With this patch series, we populate the phy_init() which was a no-op
earlier. The host controller then calls the phy_init() at the designated
place rather than doing it invariably in ufs_hcd_init().

As part of this series, we introduce phy modes for ufs phy.
The M-PHY has two data rates defined for each generations (Gears) -
Rate A and Rate B. These can serve as the two modes of ufs HS phy.
Host controller can direct the phy to set the respective configurations
based on the phy modes.

The patch-series has been tested with necessary dt patches on db820c.

Vivek Gautam (5):
  dt-bindings: phy: Add PHY_TYPE_UFS definition
  phy: Add UFS PHY modes
  phy: qcom-ufs: Add support to set phy mode
  scsi/ufs: qcom: Set phy mode based on the controllers HS MODE
  ufs/phy: qcom: Refactor to use phy_init call

 drivers/phy/qualcomm/phy-qcom-ufs-i.h        |  4 +--
 drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c | 23 ++++++++++++++--
 drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c | 23 ++++++++++++++--
 drivers/phy/qualcomm/phy-qcom-ufs.c          | 38 +++++++--------------------
 drivers/scsi/ufs/ufs-qcom.c                  | 39 ++++++++++++----------------
 include/dt-bindings/phy/phy.h                |  1 +
 include/linux/phy/phy-qcom-ufs.h             |  3 ---
 include/linux/phy/phy.h                      |  2 ++
 8 files changed, 74 insertions(+), 59 deletions(-)

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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/5] qcom-ufs: phy/hcd: Refactor phy initialization code

[Vivek Gautam]

Refactoring the qcom-ufs phy and host controller code to move
further towards the generic phy usage. Right now the qcom-ufs exports
a bunch of APIs that are used by the host controller to initialize
the phy.
With this patch series, we populate the phy_init() which was a no-op
earlier. The host controller then calls the phy_init() at the designated
place rather than doing it invariably in ufs_hcd_init().

As part of this series, we introduce phy modes for ufs phy.
The M-PHY has two data rates defined for each generations (Gears) -
Rate A and Rate B. These can serve as the two modes of ufs HS phy.
Host controller can direct the phy to set the respective configurations
based on the phy modes.

The patch-series has been tested with necessary dt patches on db820c.

Vivek Gautam (5):
  dt-bindings: phy: Add PHY_TYPE_UFS definition
  phy: Add UFS PHY modes
  phy: qcom-ufs: Add support to set phy mode
  scsi/ufs: qcom: Set phy mode based on the controllers HS MODE
  ufs/phy: qcom: Refactor to use phy_init call

 drivers/phy/qualcomm/phy-qcom-ufs-i.h        |  4 +--
 drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c | 23 ++++++++++++++--
 drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c | 23 ++++++++++++++--
 drivers/phy/qualcomm/phy-qcom-ufs.c          | 38 +++++++--------------------
 drivers/scsi/ufs/ufs-qcom.c                  | 39 ++++++++++++----------------
 include/dt-bindings/phy/phy.h                |  1 +
 include/linux/phy/phy-qcom-ufs.h             |  3 ---
 include/linux/phy/phy.h                      |  2 ++
 8 files changed, 74 insertions(+), 59 deletions(-)

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

[PATCH 1/5] dt-bindings: phy: Add PHY_TYPE_UFS definition

[Vivek Gautam]

Add definition for UFS phy type.

Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
---
 include/dt-bindings/phy/phy.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/dt-bindings/phy/phy.h b/include/dt-bindings/phy/phy.h
index 6c901930eb3e..d16e8755f6a9 100644
--- a/include/dt-bindings/phy/phy.h
+++ b/include/dt-bindings/phy/phy.h
@@ -15,5 +15,6 @@
 #define PHY_TYPE_PCIE		2
 #define PHY_TYPE_USB2		3
 #define PHY_TYPE_USB3		4
+#define PHY_TYPE_UFS		5
 
 #endif /* _DT_BINDINGS_PHY */
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 2/5] phy: Add UFS PHY modes

[Vivek Gautam]

UFS phy has two modes for each High speed generation.
These modes are identified by two rates of operations -
Rate A, and Rate B.
Add these UFS phy modes to phy framework.

Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
---
 include/linux/phy/phy.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 78bb0d7f6b11..27e25bb78bfb 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -27,6 +27,8 @@ enum phy_mode {
 	PHY_MODE_USB_HOST,
 	PHY_MODE_USB_DEVICE,
 	PHY_MODE_USB_OTG,
+	PHY_MODE_UFS_HS_A,
+	PHY_MODE_UFS_HS_B,
 };
 
 /**
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 3/5] phy: qcom-ufs: Add support to set phy mode

[Vivek Gautam]

Adding support to set desired UFS phy mode that can be set
from the host controller.

Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
---
 drivers/phy/qualcomm/phy-qcom-ufs-i.h        |  2 ++
 drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c | 14 ++++++++++++++
 drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c | 14 ++++++++++++++
 3 files changed, 30 insertions(+)

diff --git a/drivers/phy/qualcomm/phy-qcom-ufs-i.h b/drivers/phy/qualcomm/phy-qcom-ufs-i.h
index 13b02b7de30b..94326ed107c3 100644
--- a/drivers/phy/qualcomm/phy-qcom-ufs-i.h
+++ b/drivers/phy/qualcomm/phy-qcom-ufs-i.h
@@ -115,6 +115,8 @@ struct ufs_qcom_phy {
 	int cached_regs_table_size;
 	bool is_powered_on;
 	struct ufs_qcom_phy_specific_ops *phy_spec_ops;
+
+	enum phy_mode mode;
 };
 
 /**
diff --git a/drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c b/drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c
index 12a1b498dc4b..af65785230b5 100644
--- a/drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c
+++ b/drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c
@@ -53,6 +53,19 @@ static int ufs_qcom_phy_qmp_14nm_exit(struct phy *generic_phy)
 }
 
 static
+int ufs_qcom_phy_qmp_14nm_set_mode(struct phy *generic_phy, enum phy_mode mode)
+{
+	struct ufs_qcom_phy *phy_common = get_ufs_qcom_phy(generic_phy);
+
+	phy_common->mode = PHY_MODE_INVALID;
+
+	if (mode > 0)
+		phy_common->mode = mode;
+
+	return 0;
+}
+
+static
 void ufs_qcom_phy_qmp_14nm_power_control(struct ufs_qcom_phy *phy, bool val)
 {
 	writel_relaxed(val ? 0x1 : 0x0, phy->mmio + UFS_PHY_POWER_DOWN_CONTROL);
@@ -102,6 +115,7 @@ static int ufs_qcom_phy_qmp_14nm_is_pcs_ready(struct ufs_qcom_phy *phy_common)
 	.exit		= ufs_qcom_phy_qmp_14nm_exit,
 	.power_on	= ufs_qcom_phy_power_on,
 	.power_off	= ufs_qcom_phy_power_off,
+	.set_mode	= ufs_qcom_phy_qmp_14nm_set_mode,
 	.owner		= THIS_MODULE,
 };
 
diff --git a/drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c b/drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c
index 4f68acb58b73..5c18c41dbdb4 100644
--- a/drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c
+++ b/drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c
@@ -72,6 +72,19 @@ static int ufs_qcom_phy_qmp_20nm_exit(struct phy *generic_phy)
 }
 
 static
+int ufs_qcom_phy_qmp_20nm_set_mode(struct phy *generic_phy, enum phy_mode mode)
+{
+	struct ufs_qcom_phy *phy_common = get_ufs_qcom_phy(generic_phy);
+
+	phy_common->mode = PHY_MODE_INVALID;
+
+	if (mode > 0)
+		phy_common->mode = mode;
+
+	return 0;
+}
+
+static
 void ufs_qcom_phy_qmp_20nm_power_control(struct ufs_qcom_phy *phy, bool val)
 {
 	bool hibern8_exit_after_pwr_collapse = phy->quirks &
@@ -160,6 +173,7 @@ static int ufs_qcom_phy_qmp_20nm_is_pcs_ready(struct ufs_qcom_phy *phy_common)
 	.exit		= ufs_qcom_phy_qmp_20nm_exit,
 	.power_on	= ufs_qcom_phy_power_on,
 	.power_off	= ufs_qcom_phy_power_off,
+	.set_mode	= ufs_qcom_phy_qmp_20nm_set_mode,
 	.owner		= THIS_MODULE,
 };
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 4/5] scsi/ufs: qcom: Set phy mode based on the controllers HS MODE

[Vivek Gautam]

Set the phy mode based on the UFS HS PA mode. This lets the
controller let phy know the mode in which the PHY Adapter is
running and set the phy rates accordingly.

Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
---
 drivers/scsi/ufs/ufs-qcom.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index c87d770b519a..44c21d5818ee 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -273,6 +273,9 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba)
 	bool is_rate_B = (UFS_QCOM_LIMIT_HS_RATE == PA_HS_MODE_B)
 							? true : false;
 
+	if (is_rate_B)
+		phy_set_mode(phy, PHY_MODE_UFS_HS_B);
+
 	/* Assert PHY reset and apply PHY calibration values */
 	ufs_qcom_assert_reset(hba);
 	/* provide 1ms delay to let the reset pulse propagate */
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 5/5] ufs/phy: qcom: Refactor to use phy_init call

[Vivek Gautam]

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 <vivek.gautam@codeaurora.org>
---
 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;
+
 	err = ufs_qcom_phy_enable_vreg(dev, &phy_common->vdda_phy);
 	if (err) {
 		dev_err(dev, "%s enable vdda_phy failed, err=%d\n",
diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 44c21d5818ee..890eafeb8ad4 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -281,10 +281,10 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba)
 	/* provide 1ms delay to let the reset pulse propagate */
 	usleep_range(1000, 1100);
 
-	ret = ufs_qcom_phy_calibrate_phy(phy, is_rate_B);
-
+	/* phy initialization - calibrate the phy */
+	ret = phy_init(phy);
 	if (ret) {
-		dev_err(hba->dev, "%s: ufs_qcom_phy_calibrate_phy() failed, ret = %d\n",
+		dev_err(hba->dev, "%s: phy init failed, ret = %d\n",
 			__func__, ret);
 		goto out;
 	}
@@ -297,21 +297,22 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba)
 	 * voltage, current to settle down before starting serdes.
 	 */
 	usleep_range(1000, 1100);
-	ret = ufs_qcom_phy_start_serdes(phy);
+
+	/* power on phy - start serdes and phy's power and clocks */
+	ret = phy_power_on(phy);
 	if (ret) {
-		dev_err(hba->dev, "%s: ufs_qcom_phy_start_serdes() failed, ret = %d\n",
+		dev_err(hba->dev, "%s: phy power on failed, ret = %d\n",
 			__func__, ret);
-		goto out;
+		goto out_disable_phy;
 	}
 
-	ret = ufs_qcom_phy_is_pcs_ready(phy);
-	if (ret)
-		dev_err(hba->dev,
-			"%s: is_physical_coding_sublayer_ready() failed, ret = %d\n",
-			__func__, ret);
-
 	ufs_qcom_select_unipro_mode(host);
 
+	return 0;
+
+out_disable_phy:
+	ufs_qcom_assert_reset(hba);
+	phy_exit(phy);
 out:
 	return ret;
 }
@@ -1276,14 +1277,9 @@ static int ufs_qcom_init(struct ufs_hba *hba)
 	ufs_qcom_phy_save_controller_version(host->generic_phy,
 		host->hw_ver.major, host->hw_ver.minor, host->hw_ver.step);
 
-	phy_init(host->generic_phy);
-	err = phy_power_on(host->generic_phy);
-	if (err)
-		goto out_unregister_bus;
-
 	err = ufs_qcom_init_lane_clks(host);
 	if (err)
-		goto out_disable_phy;
+		goto out_variant_clear;
 
 	ufs_qcom_set_caps(hba);
 	ufs_qcom_advertise_quirks(hba);
@@ -1304,10 +1300,6 @@ static int ufs_qcom_init(struct ufs_hba *hba)
 
 	goto out;
 
-out_disable_phy:
-	phy_power_off(host->generic_phy);
-out_unregister_bus:
-	phy_exit(host->generic_phy);
 out_variant_clear:
 	ufshcd_set_variant(hba, NULL);
 out:
diff --git a/include/linux/phy/phy-qcom-ufs.h b/include/linux/phy/phy-qcom-ufs.h
index 35c070ea6ea3..0a2c18a9771d 100644
--- a/include/linux/phy/phy-qcom-ufs.h
+++ b/include/linux/phy/phy-qcom-ufs.h
@@ -31,10 +31,7 @@
  */
 void ufs_qcom_phy_disable_dev_ref_clk(struct phy *phy);
 
-int ufs_qcom_phy_start_serdes(struct phy *phy);
 int ufs_qcom_phy_set_tx_lane_enable(struct phy *phy, u32 tx_lanes);
-int ufs_qcom_phy_calibrate_phy(struct phy *phy, bool is_rate_B);
-int ufs_qcom_phy_is_pcs_ready(struct phy *phy);
 void ufs_qcom_phy_save_controller_version(struct phy *phy,
 			u8 major, u16 minor, u16 step);
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 0/5] qcom-ufs: phy/hcd: Refactor phy initialization code

[Kishon Vijay Abraham I]

Hi,

On Friday 04 August 2017 12:18 PM, Vivek Gautam wrote:
> Refactoring the qcom-ufs phy and host controller code to move
> further towards the generic phy usage. Right now the qcom-ufs exports
> a bunch of APIs that are used by the host controller to initialize
> the phy.
> With this patch series, we populate the phy_init() which was a no-op
> earlier. The host controller then calls the phy_init() at the designated
> place rather than doing it invariably in ufs_hcd_init().
> 
> As part of this series, we introduce phy modes for ufs phy.
> The M-PHY has two data rates defined for each generations (Gears) -
> Rate A and Rate B. These can serve as the two modes of ufs HS phy.
> Host controller can direct the phy to set the respective configurations
> based on the phy modes.
> 
> The patch-series has been tested with necessary dt patches on db820c.

Can the first 3 patches go independently of the other 2 or should all this be
merged together?

Thanks
Kishon

Re: [PATCH 0/5] qcom-ufs: phy/hcd: Refactor phy initialization code

[Kishon Vijay Abraham I]

Hi,

On Friday 04 August 2017 12:18 PM, Vivek Gautam wrote:
> Refactoring the qcom-ufs phy and host controller code to move
> further towards the generic phy usage. Right now the qcom-ufs exports
> a bunch of APIs that are used by the host controller to initialize
> the phy.
> With this patch series, we populate the phy_init() which was a no-op
> earlier. The host controller then calls the phy_init() at the designated
> place rather than doing it invariably in ufs_hcd_init().
> 
> As part of this series, we introduce phy modes for ufs phy.
> The M-PHY has two data rates defined for each generations (Gears) -
> Rate A and Rate B. These can serve as the two modes of ufs HS phy.
> Host controller can direct the phy to set the respective configurations
> based on the phy modes.
> 
> The patch-series has been tested with necessary dt patches on db820c.

Can the first 3 patches go independently of the other 2 or should all this be
merged together?

Thanks
Kishon

Re: [PATCH 0/5] qcom-ufs: phy/hcd: Refactor phy initialization code

[Vivek Gautam]

Hi Koshon,

On 2017-08-08 17:39, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Friday 04 August 2017 12:18 PM, Vivek Gautam wrote:
>> Refactoring the qcom-ufs phy and host controller code to move
>> further towards the generic phy usage. Right now the qcom-ufs exports
>> a bunch of APIs that are used by the host controller to initialize
>> the phy.
>> With this patch series, we populate the phy_init() which was a no-op
>> earlier. The host controller then calls the phy_init() at the 
>> designated
>> place rather than doing it invariably in ufs_hcd_init().
>> 
>> As part of this series, we introduce phy modes for ufs phy.
>> The M-PHY has two data rates defined for each generations (Gears) -
>> Rate A and Rate B. These can serve as the two modes of ufs HS phy.
>> Host controller can direct the phy to set the respective 
>> configurations
>> based on the phy modes.
>> 
>> The patch-series has been tested with necessary dt patches on db820c.
> 
> Can the first 3 patches go independently of the other 2 or should all 
> this be
> merged together?

The first 3 patches are independent, but the next 2 patches depend on 
those 3 for functionality.
I would prefer all to go in one tree. If you want to pull these in the 
phy tree,
I will request Subhash/Martin to ack the patches.


Regards
Vivek

> 
> Thanks
> Kishon
> --
> To unsubscribe from this list: send the line "unsubscribe 
> linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] qcom-ufs: phy/hcd: Refactor phy initialization code

[Kishon Vijay Abraham I]

Vivek,

On Tuesday 08 August 2017 09:20 PM, Vivek Gautam wrote:
> Hi Koshon,
> 
> On 2017-08-08 17:39, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Friday 04 August 2017 12:18 PM, Vivek Gautam wrote:
>>> Refactoring the qcom-ufs phy and host controller code to move
>>> further towards the generic phy usage. Right now the qcom-ufs exports
>>> a bunch of APIs that are used by the host controller to initialize
>>> the phy.
>>> With this patch series, we populate the phy_init() which was a no-op
>>> earlier. The host controller then calls the phy_init() at the designated
>>> place rather than doing it invariably in ufs_hcd_init().
>>>
>>> As part of this series, we introduce phy modes for ufs phy.
>>> The M-PHY has two data rates defined for each generations (Gears) -
>>> Rate A and Rate B. These can serve as the two modes of ufs HS phy.
>>> Host controller can direct the phy to set the respective configurations
>>> based on the phy modes.
>>>
>>> The patch-series has been tested with necessary dt patches on db820c.
>>
>> Can the first 3 patches go independently of the other 2 or should all this be
>> merged together?
> 
> The first 3 patches are independent, but the next 2 patches depend on those 3
> for functionality.
> I would prefer all to go in one tree. If you want to pull these in the phy tree,
> I will request Subhash/Martin to ack the patches.

sure, that should be fine!

Thanks
Kishon

Re: [PATCH 0/5] qcom-ufs: phy/hcd: Refactor phy initialization code

[Kishon Vijay Abraham I]

Vivek,

On Tuesday 08 August 2017 09:20 PM, Vivek Gautam wrote:
> Hi Koshon,
> 
> On 2017-08-08 17:39, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Friday 04 August 2017 12:18 PM, Vivek Gautam wrote:
>>> Refactoring the qcom-ufs phy and host controller code to move
>>> further towards the generic phy usage. Right now the qcom-ufs exports
>>> a bunch of APIs that are used by the host controller to initialize
>>> the phy.
>>> With this patch series, we populate the phy_init() which was a no-op
>>> earlier. The host controller then calls the phy_init() at the designated
>>> place rather than doing it invariably in ufs_hcd_init().
>>>
>>> As part of this series, we introduce phy modes for ufs phy.
>>> The M-PHY has two data rates defined for each generations (Gears) -
>>> Rate A and Rate B. These can serve as the two modes of ufs HS phy.
>>> Host controller can direct the phy to set the respective configurations
>>> based on the phy modes.
>>>
>>> The patch-series has been tested with necessary dt patches on db820c.
>>
>> Can the first 3 patches go independently of the other 2 or should all this be
>> merged together?
> 
> The first 3 patches are independent, but the next 2 patches depend on those 3
> for functionality.
> I would prefer all to go in one tree. If you want to pull these in the phy tree,
> I will request Subhash/Martin to ack the patches.

sure, that should be fine!

Thanks
Kishon

Re: [PATCH 0/5] qcom-ufs: phy/hcd: Refactor phy initialization code

[Vivek Gautam]

Hi Martin, Subhash


On Wed, Aug 9, 2017 at 11:18 AM, Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org> wrote:
> Vivek,
>
> On Tuesday 08 August 2017 09:20 PM, Vivek Gautam wrote:
>> Hi Koshon,
>>
>> On 2017-08-08 17:39, Kishon Vijay Abraham I wrote:
>>> Hi,
>>>
>>> On Friday 04 August 2017 12:18 PM, Vivek Gautam wrote:
>>>> Refactoring the qcom-ufs phy and host controller code to move
>>>> further towards the generic phy usage. Right now the qcom-ufs exports
>>>> a bunch of APIs that are used by the host controller to initialize
>>>> the phy.
>>>> With this patch series, we populate the phy_init() which was a no-op
>>>> earlier. The host controller then calls the phy_init() at the designated
>>>> place rather than doing it invariably in ufs_hcd_init().
>>>>
>>>> As part of this series, we introduce phy modes for ufs phy.
>>>> The M-PHY has two data rates defined for each generations (Gears) -
>>>> Rate A and Rate B. These can serve as the two modes of ufs HS phy.
>>>> Host controller can direct the phy to set the respective configurations
>>>> based on the phy modes.
>>>>
>>>> The patch-series has been tested with necessary dt patches on db820c.
>>>
>>> Can the first 3 patches go independently of the other 2 or should all this be
>>> merged together?
>>
>> The first 3 patches are independent, but the next 2 patches depend on those 3
>> for functionality.
>> I would prefer all to go in one tree. If you want to pull these in the phy tree,
>> I will request Subhash/Martin to ack the patches.

Can you kindly review this patch series (for UFS controller changes) and
consider giving your Ack so that Kishon can pull in the series through phy tree.
Thanks.

best regards
Vivek

>
> sure, that should be fine!
>
> Thanks
> Kishon
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] qcom-ufs: phy/hcd: Refactor phy initialization code

[Vivek Gautam]

Hi Martin, Subhash


On Wed, Aug 9, 2017 at 11:18 AM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
> Vivek,
>
> On Tuesday 08 August 2017 09:20 PM, Vivek Gautam wrote:
>> Hi Koshon,
>>
>> On 2017-08-08 17:39, Kishon Vijay Abraham I wrote:
>>> Hi,
>>>
>>> On Friday 04 August 2017 12:18 PM, Vivek Gautam wrote:
>>>> Refactoring the qcom-ufs phy and host controller code to move
>>>> further towards the generic phy usage. Right now the qcom-ufs exports
>>>> a bunch of APIs that are used by the host controller to initialize
>>>> the phy.
>>>> With this patch series, we populate the phy_init() which was a no-op
>>>> earlier. The host controller then calls the phy_init() at the designated
>>>> place rather than doing it invariably in ufs_hcd_init().
>>>>
>>>> As part of this series, we introduce phy modes for ufs phy.
>>>> The M-PHY has two data rates defined for each generations (Gears) -
>>>> Rate A and Rate B. These can serve as the two modes of ufs HS phy.
>>>> Host controller can direct the phy to set the respective configurations
>>>> based on the phy modes.
>>>>
>>>> The patch-series has been tested with necessary dt patches on db820c.
>>>
>>> Can the first 3 patches go independently of the other 2 or should all this be
>>> merged together?
>>
>> The first 3 patches are independent, but the next 2 patches depend on those 3
>> for functionality.
>> I would prefer all to go in one tree. If you want to pull these in the phy tree,
>> I will request Subhash/Martin to ack the patches.

Can you kindly review this patch series (for UFS controller changes) and
consider giving your Ack so that Kishon can pull in the series through phy tree.
Thanks.

best regards
Vivek

>
> sure, that should be fine!
>
> Thanks
> Kishon
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 0/5] qcom-ufs: phy/hcd: Refactor phy initialization code

[Martin K. Petersen]

Vivek,

> Can you kindly review this patch series (for UFS controller changes)
> and consider giving your Ack so that Kishon can pull in the series
> through phy tree.

SCSI piece looks OK.

Would still like Subhash to review the rest.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 0/5] qcom-ufs: phy/hcd: Refactor phy initialization code

[Martin K. Petersen]

Vivek,

> Can you kindly review this patch series (for UFS controller changes)
> and consider giving your Ack so that Kishon can pull in the series
> through phy tree.

SCSI piece looks OK.

Would still like Subhash to review the rest.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 0/5] qcom-ufs: phy/hcd: Refactor phy initialization code

[Vivek Gautam]

On Fri, Aug 11, 2017 at 5:33 AM, Martin K. Petersen
<martin.petersen@oracle.com> wrote:
>
> Vivek,
>
>> Can you kindly review this patch series (for UFS controller changes)
>> and consider giving your Ack so that Kishon can pull in the series
>> through phy tree.
>
> SCSI piece looks OK.

Thank you Martin for your review.
>
> Would still like Subhash to review the rest.

Subhash is on vacation with limited access to emails. I will ask
his team to take a look.

regards
Vivek

>
> --
> Martin K. Petersen      Oracle Linux Engineering
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 0/5] qcom-ufs: phy/hcd: Refactor phy initialization code

[Asutosh Das (asd)]

Vivek,

On 8/11/2017 12:42 PM, Vivek Gautam wrote:
> On Fri, Aug 11, 2017 at 5:33 AM, Martin K. Petersen
> <martin.petersen@oracle.com> wrote:
>>
>> Vivek,
>>
>>> Can you kindly review this patch series (for UFS controller changes)
>>> and consider giving your Ack so that Kishon can pull in the series
>>> through phy tree.
>>
>> SCSI piece looks OK.
> 
> Thank you Martin for your review.
>>
>> Would still like Subhash to review the rest.
> 
> Subhash is on vacation with limited access to emails. I will ask
> his team to take a look.
> 
Looks good to me.
> regards
> Vivek
> 
>>
>> --
>> Martin K. Petersen      Oracle Linux Engineering
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 

-- 
Asutosh Das (asd)
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a 
Linux Foundation Collaborative Project

Re: [PATCH 4/5] scsi/ufs: qcom: Set phy mode based on the controllers HS MODE

[Subhash Jadavani]

On 2017-08-03 23:48, Vivek Gautam wrote:
> Set the phy mode based on the UFS HS PA mode. This lets the
> controller let phy know the mode in which the PHY Adapter is
> running and set the phy rates accordingly.
> 
> Signed-off-by: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
>  drivers/scsi/ufs/ufs-qcom.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
> index c87d770b519a..44c21d5818ee 100644
> --- a/drivers/scsi/ufs/ufs-qcom.c
> +++ b/drivers/scsi/ufs/ufs-qcom.c
> @@ -273,6 +273,9 @@ static int ufs_qcom_power_up_sequence(struct 
> ufs_hba *hba)
>  	bool is_rate_B = (UFS_QCOM_LIMIT_HS_RATE == PA_HS_MODE_B)
>  							? true : false;
> 
> +	if (is_rate_B)
> +		phy_set_mode(phy, PHY_MODE_UFS_HS_B);
> +
>  	/* Assert PHY reset and apply PHY calibration values */
>  	ufs_qcom_assert_reset(hba);
>  	/* provide 1ms delay to let the reset pulse propagate */

Looks good to me.
Reviewed-by: Subhash Jadavani <subhashj-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] scsi/ufs: qcom: Set phy mode based on the controllers HS MODE

[Subhash Jadavani]

On 2017-08-03 23:48, Vivek Gautam wrote:
> Set the phy mode based on the UFS HS PA mode. This lets the
> controller let phy know the mode in which the PHY Adapter is
> running and set the phy rates accordingly.
> 
> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
> ---
>  drivers/scsi/ufs/ufs-qcom.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
> index c87d770b519a..44c21d5818ee 100644
> --- a/drivers/scsi/ufs/ufs-qcom.c
> +++ b/drivers/scsi/ufs/ufs-qcom.c
> @@ -273,6 +273,9 @@ static int ufs_qcom_power_up_sequence(struct 
> ufs_hba *hba)
>  	bool is_rate_B = (UFS_QCOM_LIMIT_HS_RATE == PA_HS_MODE_B)
>  							? true : false;
> 
> +	if (is_rate_B)
> +		phy_set_mode(phy, PHY_MODE_UFS_HS_B);
> +
>  	/* Assert PHY reset and apply PHY calibration values */
>  	ufs_qcom_assert_reset(hba);
>  	/* provide 1ms delay to let the reset pulse propagate */

Looks good to me.
Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 5/5] ufs/phy: qcom: Refactor to use phy_init call

[Subhash Jadavani]

Hi Vivek,

Please find one comment inline below, rest look good.

Regards,
Subhash

On 2017-08-03 23:48, Vivek Gautam wrote:
> 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 <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
>  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.


>  	err = ufs_qcom_phy_enable_vreg(dev, &phy_common->vdda_phy);
>  	if (err) {
>  		dev_err(dev, "%s enable vdda_phy failed, err=%d\n",
> diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
> index 44c21d5818ee..890eafeb8ad4 100644
> --- a/drivers/scsi/ufs/ufs-qcom.c
> +++ b/drivers/scsi/ufs/ufs-qcom.c
> @@ -281,10 +281,10 @@ static int ufs_qcom_power_up_sequence(struct 
> ufs_hba *hba)
>  	/* provide 1ms delay to let the reset pulse propagate */
>  	usleep_range(1000, 1100);
> 
> -	ret = ufs_qcom_phy_calibrate_phy(phy, is_rate_B);
> -
> +	/* phy initialization - calibrate the phy */
> +	ret = phy_init(phy);
>  	if (ret) {
> -		dev_err(hba->dev, "%s: ufs_qcom_phy_calibrate_phy() failed, ret = 
> %d\n",
> +		dev_err(hba->dev, "%s: phy init failed, ret = %d\n",
>  			__func__, ret);
>  		goto out;
>  	}
> @@ -297,21 +297,22 @@ static int ufs_qcom_power_up_sequence(struct 
> ufs_hba *hba)
>  	 * voltage, current to settle down before starting serdes.
>  	 */
>  	usleep_range(1000, 1100);
> -	ret = ufs_qcom_phy_start_serdes(phy);
> +
> +	/* power on phy - start serdes and phy's power and clocks */
> +	ret = phy_power_on(phy);
>  	if (ret) {
> -		dev_err(hba->dev, "%s: ufs_qcom_phy_start_serdes() failed, ret = 
> %d\n",
> +		dev_err(hba->dev, "%s: phy power on failed, ret = %d\n",
>  			__func__, ret);
> -		goto out;
> +		goto out_disable_phy;
>  	}
> 
> -	ret = ufs_qcom_phy_is_pcs_ready(phy);
> -	if (ret)
> -		dev_err(hba->dev,
> -			"%s: is_physical_coding_sublayer_ready() failed, ret = %d\n",
> -			__func__, ret);
> -
>  	ufs_qcom_select_unipro_mode(host);
> 
> +	return 0;
> +
> +out_disable_phy:
> +	ufs_qcom_assert_reset(hba);
> +	phy_exit(phy);
>  out:
>  	return ret;
>  }
> @@ -1276,14 +1277,9 @@ static int ufs_qcom_init(struct ufs_hba *hba)
>  	ufs_qcom_phy_save_controller_version(host->generic_phy,
>  		host->hw_ver.major, host->hw_ver.minor, host->hw_ver.step);
> 
> -	phy_init(host->generic_phy);
> -	err = phy_power_on(host->generic_phy);
> -	if (err)
> -		goto out_unregister_bus;
> -
>  	err = ufs_qcom_init_lane_clks(host);
>  	if (err)
> -		goto out_disable_phy;
> +		goto out_variant_clear;
> 
>  	ufs_qcom_set_caps(hba);
>  	ufs_qcom_advertise_quirks(hba);
> @@ -1304,10 +1300,6 @@ static int ufs_qcom_init(struct ufs_hba *hba)
> 
>  	goto out;
> 
> -out_disable_phy:
> -	phy_power_off(host->generic_phy);
> -out_unregister_bus:
> -	phy_exit(host->generic_phy);
>  out_variant_clear:
>  	ufshcd_set_variant(hba, NULL);
>  out:
> diff --git a/include/linux/phy/phy-qcom-ufs.h 
> b/include/linux/phy/phy-qcom-ufs.h
> index 35c070ea6ea3..0a2c18a9771d 100644
> --- a/include/linux/phy/phy-qcom-ufs.h
> +++ b/include/linux/phy/phy-qcom-ufs.h
> @@ -31,10 +31,7 @@
>   */
>  void ufs_qcom_phy_disable_dev_ref_clk(struct phy *phy);
> 
> -int ufs_qcom_phy_start_serdes(struct phy *phy);
>  int ufs_qcom_phy_set_tx_lane_enable(struct phy *phy, u32 tx_lanes);
> -int ufs_qcom_phy_calibrate_phy(struct phy *phy, bool is_rate_B);
> -int ufs_qcom_phy_is_pcs_ready(struct phy *phy);
>  void ufs_qcom_phy_save_controller_version(struct phy *phy,
>  			u8 major, u16 minor, u16 step);

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] ufs/phy: qcom: Refactor to use phy_init call

[Subhash Jadavani]

Hi Vivek,

Please find one comment inline below, rest look good.

Regards,
Subhash

On 2017-08-03 23:48, Vivek Gautam wrote:
> 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 <vivek.gautam@codeaurora.org>
> ---
>  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.


>  	err = ufs_qcom_phy_enable_vreg(dev, &phy_common->vdda_phy);
>  	if (err) {
>  		dev_err(dev, "%s enable vdda_phy failed, err=%d\n",
> diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
> index 44c21d5818ee..890eafeb8ad4 100644
> --- a/drivers/scsi/ufs/ufs-qcom.c
> +++ b/drivers/scsi/ufs/ufs-qcom.c
> @@ -281,10 +281,10 @@ static int ufs_qcom_power_up_sequence(struct 
> ufs_hba *hba)
>  	/* provide 1ms delay to let the reset pulse propagate */
>  	usleep_range(1000, 1100);
> 
> -	ret = ufs_qcom_phy_calibrate_phy(phy, is_rate_B);
> -
> +	/* phy initialization - calibrate the phy */
> +	ret = phy_init(phy);
>  	if (ret) {
> -		dev_err(hba->dev, "%s: ufs_qcom_phy_calibrate_phy() failed, ret = 
> %d\n",
> +		dev_err(hba->dev, "%s: phy init failed, ret = %d\n",
>  			__func__, ret);
>  		goto out;
>  	}
> @@ -297,21 +297,22 @@ static int ufs_qcom_power_up_sequence(struct 
> ufs_hba *hba)
>  	 * voltage, current to settle down before starting serdes.
>  	 */
>  	usleep_range(1000, 1100);
> -	ret = ufs_qcom_phy_start_serdes(phy);
> +
> +	/* power on phy - start serdes and phy's power and clocks */
> +	ret = phy_power_on(phy);
>  	if (ret) {
> -		dev_err(hba->dev, "%s: ufs_qcom_phy_start_serdes() failed, ret = 
> %d\n",
> +		dev_err(hba->dev, "%s: phy power on failed, ret = %d\n",
>  			__func__, ret);
> -		goto out;
> +		goto out_disable_phy;
>  	}
> 
> -	ret = ufs_qcom_phy_is_pcs_ready(phy);
> -	if (ret)
> -		dev_err(hba->dev,
> -			"%s: is_physical_coding_sublayer_ready() failed, ret = %d\n",
> -			__func__, ret);
> -
>  	ufs_qcom_select_unipro_mode(host);
> 
> +	return 0;
> +
> +out_disable_phy:
> +	ufs_qcom_assert_reset(hba);
> +	phy_exit(phy);
>  out:
>  	return ret;
>  }
> @@ -1276,14 +1277,9 @@ static int ufs_qcom_init(struct ufs_hba *hba)
>  	ufs_qcom_phy_save_controller_version(host->generic_phy,
>  		host->hw_ver.major, host->hw_ver.minor, host->hw_ver.step);
> 
> -	phy_init(host->generic_phy);
> -	err = phy_power_on(host->generic_phy);
> -	if (err)
> -		goto out_unregister_bus;
> -
>  	err = ufs_qcom_init_lane_clks(host);
>  	if (err)
> -		goto out_disable_phy;
> +		goto out_variant_clear;
> 
>  	ufs_qcom_set_caps(hba);
>  	ufs_qcom_advertise_quirks(hba);
> @@ -1304,10 +1300,6 @@ static int ufs_qcom_init(struct ufs_hba *hba)
> 
>  	goto out;
> 
> -out_disable_phy:
> -	phy_power_off(host->generic_phy);
> -out_unregister_bus:
> -	phy_exit(host->generic_phy);
>  out_variant_clear:
>  	ufshcd_set_variant(hba, NULL);
>  out:
> diff --git a/include/linux/phy/phy-qcom-ufs.h 
> b/include/linux/phy/phy-qcom-ufs.h
> index 35c070ea6ea3..0a2c18a9771d 100644
> --- a/include/linux/phy/phy-qcom-ufs.h
> +++ b/include/linux/phy/phy-qcom-ufs.h
> @@ -31,10 +31,7 @@
>   */
>  void ufs_qcom_phy_disable_dev_ref_clk(struct phy *phy);
> 
> -int ufs_qcom_phy_start_serdes(struct phy *phy);
>  int ufs_qcom_phy_set_tx_lane_enable(struct phy *phy, u32 tx_lanes);
> -int ufs_qcom_phy_calibrate_phy(struct phy *phy, bool is_rate_B);
> -int ufs_qcom_phy_is_pcs_ready(struct phy *phy);
>  void ufs_qcom_phy_save_controller_version(struct phy *phy,
>  			u8 major, u16 minor, u16 step);

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 5/5] ufs/phy: qcom: Refactor to use phy_init call

[Vivek Gautam]

Hi Subhash,


On Wed, Sep 27, 2017 at 4:43 AM, Subhash Jadavani
<subhashj@codeaurora.org> wrote:
> Hi Vivek,
>
> Please find one comment inline below, rest look good.
>
> Regards,
> Subhash
>
>
> On 2017-08-03 23:48, Vivek Gautam wrote:
>>
>> 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 <vivek.gautam@codeaurora.org>
>> ---
>>  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

Re: [PATCH 4/5] scsi/ufs: qcom: Set phy mode based on the controllers HS MODE

[Vivek Gautam]

On 09/27/2017 04:14 AM, Subhash Jadavani wrote:
> On 2017-08-03 23:48, Vivek Gautam wrote:
>> Set the phy mode based on the UFS HS PA mode. This lets the
>> controller let phy know the mode in which the PHY Adapter is
>> running and set the phy rates accordingly.
>>
>> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
>> ---
>>  drivers/scsi/ufs/ufs-qcom.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
>> index c87d770b519a..44c21d5818ee 100644
>> --- a/drivers/scsi/ufs/ufs-qcom.c
>> +++ b/drivers/scsi/ufs/ufs-qcom.c
>> @@ -273,6 +273,9 @@ static int ufs_qcom_power_up_sequence(struct 
>> ufs_hba *hba)
>>      bool is_rate_B = (UFS_QCOM_LIMIT_HS_RATE == PA_HS_MODE_B)
>>                              ? true : false;
>>
>> +    if (is_rate_B)
>> +        phy_set_mode(phy, PHY_MODE_UFS_HS_B);
>> +
>>      /* Assert PHY reset and apply PHY calibration values */
>>      ufs_qcom_assert_reset(hba);
>>      /* provide 1ms delay to let the reset pulse propagate */
>
> Looks good to me.
> Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>

Thanks for reviewing Subhash.

BRs
Vivek

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project