[PATCH V6 00/10] Refactor ufs phy powerup sequence

[PATCH V6 00/10] Refactor ufs phy powerup sequence

[Nitin Rawat]

In Current code regulators enable, clks enable, calibrating UFS PHY,
start_serdes and polling PCS_ready_status are part of phy_power_on.

UFS PHY registers are retained after power collapse, meaning calibrating
UFS PHY, start_serdes and polling PCS_ready_status can be done only when
hba is powered_on, and not needed every time when phy_power_on is called
during resume. Hence keep the code which enables PHY's regulators & clks
in phy_power_on and move the rest steps into phy_calibrate function.

Since phy_power_on is separated out from phy calibrate, make separate calls
to phy_power_on and phy_calibrate calls from ufs qcom driver.

Also for better power saving, remove the phy_power_on/off calls from
resume/suspend path and put them to ufs_qcom_setup_clocks, so that
PHY's regulators & clks can be turned on/off along with UFS's clocks.

This patch series is tested on SM8550 QRD, SM8650 MTP , SM8750 MTP.

Note: Patch 2 of this series is a requirement for the rest of the PHY
      patches(patch 3 - patch 9) for the functional dependency and
      Patch 1 is a fix for existing issues, which doesn't have any
      dependencies on any other changes.

Changes in v6:
1. Addressed Konrad's and Manivannan's comments to move patch11
   of v5 to start of the series so that it can be applied
   separately if needed.
2. Addessed manivannan's comment to improve commit text for few patch.
   and remove few trivial comments and minor code improvement.
3. Removed patch 10 (Introduce phy_power_on/off wrapper function)
   of last patchset as it is not needed.

Changes in v5:
1. Addressed Dmitry's comment to inline qmp_ufs_exit into
   qmp_ufs_power_off
2. Addrsssed Konrad's comment to improve code identation and updating
   kernel doc to update the info regarding controller and phy clocks
   being managed together.
3. Addrsssed Konrad's comment to update commit text for patch #10 to
   improve explanation to maintain separate ref_count in ufs controller
   driver.


Changes in v4:
1. Addressed Dmitry's comment to update cover letter to mention patch1
   as a requirement for the rest of the PHY patches.
2. Addressed Dmitry's comment to move parsing UFS PHY reset handle logic
   to init from probe to avoid probe failure.
3. Addressed Dmitry's comment to update commit text to reflect reason
   to remove qmp_ufs_com_init() (Patch 7 of current series)
4. Addrssed Konrad's comment to return failure from power up sequence when
    phy_calibrate return failure and modify the debug print.

Changes in v3:
1. Addresed neil and bjorn comment to align the order of the patch to
   maintain the bisectability compliance within the patch.
2. Addressed neil comment to move qmp_ufs_get_phy_reset() in a separate
   patch, inline qmp_ufs_com_init() inline.

Changes in v2:
1. Addressed vinod koul and manivannan comment to split the phy patch
   into multiple patches.
2. Addressed vinod's comment to reuse SW_PWRDN instead of creating
   new macros SW_PWRUP in phy-qcom-qmp-ufs.c.
3. Addressed Konrad's comment to optimize mutex lock in ufs-qcom.c
4. Addressed konrad and Manivannan comment to clean debug print in
   ufs-qcom.c

Nitin Rawat (10):
  scsi: ufs: qcom: Prevent calling phy_exit before phy_init
  scsi: ufs: qcom: add a new phy calibrate API call
  phy: qcom-qmp-ufs: Rename qmp_ufs_enable and qmp_ufs_power_on
  phy: qcom-qmp-ufs: Refactor phy_power_on and phy_calibrate callbacks
  phy: qcom-qmp-ufs: Refactor UFS PHY reset
  phy: qcom-qmp-ufs: Remove qmp_ufs_com_init()
  phy: qcom-qmp-ufs: Rename qmp_ufs_power_off
  phy: qcom-qmp-ufs: Remove qmp_ufs_exit() and Inline qmp_ufs_com_exit()
  phy: qcom-qmp-ufs: refactor qmp_ufs_power_off
  scsi: ufs: qcom : Refactor phy_power_on/off calls

 drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 141 +++++++-----------------
 drivers/ufs/host/ufs-qcom.c             |  69 ++++++------
 2 files changed, 77 insertions(+), 133 deletions(-)

--
2.48.1

[PATCH V6 01/10] scsi: ufs: qcom: Prevent calling phy_exit before phy_init

[Nitin Rawat]

Prevent calling phy_exit before phy_init to avoid abnormal power
count and the following warning during boot up.

[5.146763] phy phy-1d80000.phy.0: phy_power_on was called before phy_init

Fixes: 7bac65687510 ("scsi: ufs: qcom: Power off the PHY if it was already powered on in ufs_qcom_power_up_sequence()")
Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 drivers/ufs/host/ufs-qcom.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 37887ec68412..62a5d76450b6 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -506,10 +506,9 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba)
 	if (ret)
 		return ret;
 
-	if (phy->power_count) {
+	if (phy->power_count)
 		phy_power_off(phy);
-		phy_exit(phy);
-	}
+
 
 	/* phy initialization - calibrate the phy */
 	ret = phy_init(phy);
-- 
2.48.1

[PATCH V6 02/10] scsi: ufs: qcom: add a new phy calibrate API call

[Nitin Rawat]

Introduce a new phy calibrate API call in the UFS Qualcomm driver to
separate phy calibration from phy power-on. This change is a precursor
to the successive commits in this series, which requires these two
operations to be distinct.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
---
 drivers/ufs/host/ufs-qcom.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index 62a5d76450b6..dfe164da3668 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -530,6 +530,12 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba)
 		goto out_disable_phy;
 	}

+	ret = phy_calibrate(phy);
+	if (ret) {
+		dev_err(hba->dev, "Failed to calibrate PHY: %d\n", ret);
+		goto out_disable_phy;
+	}
+
 	ufs_qcom_select_unipro_mode(host);

 	return 0;
--
2.48.1

[PATCH V6 03/10] phy: qcom-qmp-ufs: Rename qmp_ufs_enable and qmp_ufs_power_on

[Nitin Rawat]

Rename qmp_ufs_enable to qmp_ufs_power_on and qmp_ufs_power_on to
qmp_ufs_phy_calibrate to better reflect their functionality. Also
update function calls and structure assignments accordingly.

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Co-developed-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
index b33e2e2b5014..a67cf0a64f74 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
@@ -1838,7 +1838,7 @@ static int qmp_ufs_init(struct phy *phy)
 	return 0;
 }

-static int qmp_ufs_power_on(struct phy *phy)
+static int qmp_ufs_phy_calibrate(struct phy *phy)
 {
 	struct qmp_ufs *qmp = phy_get_drvdata(phy);
 	const struct qmp_phy_cfg *cfg = qmp->cfg;
@@ -1899,7 +1899,7 @@ static int qmp_ufs_exit(struct phy *phy)
 	return 0;
 }

-static int qmp_ufs_enable(struct phy *phy)
+static int qmp_ufs_power_on(struct phy *phy)
 {
 	int ret;

@@ -1907,7 +1907,7 @@ static int qmp_ufs_enable(struct phy *phy)
 	if (ret)
 		return ret;

-	ret = qmp_ufs_power_on(phy);
+	ret = qmp_ufs_phy_calibrate(phy);
 	if (ret)
 		qmp_ufs_exit(phy);

@@ -1941,7 +1941,7 @@ static int qmp_ufs_set_mode(struct phy *phy, enum phy_mode mode, int submode)
 }

 static const struct phy_ops qcom_qmp_ufs_phy_ops = {
-	.power_on	= qmp_ufs_enable,
+	.power_on	= qmp_ufs_power_on,
 	.power_off	= qmp_ufs_disable,
 	.set_mode	= qmp_ufs_set_mode,
 	.owner		= THIS_MODULE,
--
2.48.1

[PATCH V6 04/10] phy: qcom-qmp-ufs: Refactor phy_power_on and phy_calibrate callbacks

[Nitin Rawat]

Commit 052553af6a31 ("ufs/phy: qcom: Refactor to use phy_init call")
puts enabling regulators & clks, calibrating UFS PHY, starting serdes
and polling PCS ready status into phy_power_on.

In Current code regulators enable, clks enable, calibrating UFS PHY,
start_serdes and polling PCS_ready_status are part of phy_power_on.

UFS PHY registers are retained after power collapse, meaning calibrating
UFS PHY, start_serdes and polling PCS_ready_status can be done only when
hba is powered_on, and not needed every time when phy_power_on is called
during resume. Hence keep the code which enables PHY's regulators & clks
in phy_power_on and move the rest steps into phy_calibrate function.

Refactor the code to retain PHY regulators & clks in phy_power_on and
move out rest of the code to new phy_calibrate function.

Also move reset_control_assert to qmp_ufs_phy_calibrate to align
with Hardware programming guide.

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Co-developed-by: Can Guo <quic_cang@quicinc.com>
Signed-off-by: Can Guo <quic_cang@quicinc.com>
Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 26 ++++++-------------------
 1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
index a67cf0a64f74..ade8e9c4b9ae 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
@@ -1797,7 +1797,7 @@ static int qmp_ufs_com_exit(struct qmp_ufs *qmp)
 	return 0;
 }

-static int qmp_ufs_init(struct phy *phy)
+static int qmp_ufs_power_on(struct phy *phy)
 {
 	struct qmp_ufs *qmp = phy_get_drvdata(phy);
 	const struct qmp_phy_cfg *cfg = qmp->cfg;
@@ -1825,10 +1825,6 @@ static int qmp_ufs_init(struct phy *phy)
 				return ret;
 			}
 		}
-
-		ret = reset_control_assert(qmp->ufs_reset);
-		if (ret)
-			return ret;
 	}

 	ret = qmp_ufs_com_init(qmp);
@@ -1847,6 +1843,10 @@ static int qmp_ufs_phy_calibrate(struct phy *phy)
 	unsigned int val;
 	int ret;

+	ret = reset_control_assert(qmp->ufs_reset);
+	if (ret)
+		return ret;
+
 	qmp_ufs_init_registers(qmp, cfg);

 	ret = reset_control_deassert(qmp->ufs_reset);
@@ -1899,21 +1899,6 @@ static int qmp_ufs_exit(struct phy *phy)
 	return 0;
 }

-static int qmp_ufs_power_on(struct phy *phy)
-{
-	int ret;
-
-	ret = qmp_ufs_init(phy);
-	if (ret)
-		return ret;
-
-	ret = qmp_ufs_phy_calibrate(phy);
-	if (ret)
-		qmp_ufs_exit(phy);
-
-	return ret;
-}
-
 static int qmp_ufs_disable(struct phy *phy)
 {
 	int ret;
@@ -1943,6 +1928,7 @@ static int qmp_ufs_set_mode(struct phy *phy, enum phy_mode mode, int submode)
 static const struct phy_ops qcom_qmp_ufs_phy_ops = {
 	.power_on	= qmp_ufs_power_on,
 	.power_off	= qmp_ufs_disable,
+	.calibrate	= qmp_ufs_phy_calibrate,
 	.set_mode	= qmp_ufs_set_mode,
 	.owner		= THIS_MODULE,
 };
--
2.48.1

[PATCH V6 05/10] phy: qcom-qmp-ufs: Refactor UFS PHY reset

[Nitin Rawat]

Refactor the UFS PHY reset handling to parse the reset logic only once
during initialization, instead of every resume.

As part of this change, move the UFS PHY reset parsing logic from
qmp_phy_power_on to the new qmp_ufs_phy_init function introduced
as part of phy_ops::init callback.

Co-developed-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 59 +++++++++++++------------
 1 file changed, 31 insertions(+), 28 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
index ade8e9c4b9ae..33d238cf49aa 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
@@ -1800,38 +1800,11 @@ static int qmp_ufs_com_exit(struct qmp_ufs *qmp)
 static int qmp_ufs_power_on(struct phy *phy)
 {
 	struct qmp_ufs *qmp = phy_get_drvdata(phy);
-	const struct qmp_phy_cfg *cfg = qmp->cfg;
 	int ret;
 	dev_vdbg(qmp->dev, "Initializing QMP phy\n");

-	if (cfg->no_pcs_sw_reset) {
-		/*
-		 * Get UFS reset, which is delayed until now to avoid a
-		 * circular dependency where UFS needs its PHY, but the PHY
-		 * needs this UFS reset.
-		 */
-		if (!qmp->ufs_reset) {
-			qmp->ufs_reset =
-				devm_reset_control_get_exclusive(qmp->dev,
-								 "ufsphy");
-
-			if (IS_ERR(qmp->ufs_reset)) {
-				ret = PTR_ERR(qmp->ufs_reset);
-				dev_err(qmp->dev,
-					"failed to get UFS reset: %d\n",
-					ret);
-
-				qmp->ufs_reset = NULL;
-				return ret;
-			}
-		}
-	}
-
 	ret = qmp_ufs_com_init(qmp);
-	if (ret)
-		return ret;
-
-	return 0;
+	return ret;
 }

 static int qmp_ufs_phy_calibrate(struct phy *phy)
@@ -1925,7 +1898,37 @@ static int qmp_ufs_set_mode(struct phy *phy, enum phy_mode mode, int submode)
 	return 0;
 }

+static int qmp_ufs_phy_init(struct phy *phy)
+{
+	struct qmp_ufs *qmp = phy_get_drvdata(phy);
+	const struct qmp_phy_cfg *cfg = qmp->cfg;
+	int ret;
+
+	if (!cfg->no_pcs_sw_reset)
+		return 0;
+
+	/*
+	 * Get UFS reset, which is delayed until now to avoid a
+	 * circular dependency where UFS needs its PHY, but the PHY
+	 * needs this UFS reset.
+	 */
+	if (!qmp->ufs_reset) {
+		qmp->ufs_reset =
+			devm_reset_control_get_exclusive(qmp->dev, "ufsphy");
+
+		if (IS_ERR(qmp->ufs_reset)) {
+			ret = PTR_ERR(qmp->ufs_reset);
+			dev_err(qmp->dev, "failed to get PHY reset: %d\n", ret);
+			qmp->ufs_reset = NULL;
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
 static const struct phy_ops qcom_qmp_ufs_phy_ops = {
+	.init		= qmp_ufs_phy_init,
 	.power_on	= qmp_ufs_power_on,
 	.power_off	= qmp_ufs_disable,
 	.calibrate	= qmp_ufs_phy_calibrate,
--
2.48.1

[PATCH V6 06/10] phy: qcom-qmp-ufs: Remove qmp_ufs_com_init()

[Nitin Rawat]

The qmp_ufs_power_on() function acts as a wrapper, solely invoking
qmp_ufs_com_init(). Additionally, the code within qmp_ufs_com_init()
does not correspond well with its name.

Therefore, to enhance the readability and eliminate unnecessary
function call inline qmp_ufs_com_init() into qmp_ufs_power_on().

There is no change to the functionality.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 41 +++++++++----------------
 1 file changed, 15 insertions(+), 26 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
index 33d238cf49aa..eda0a59918ea 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
@@ -1758,9 +1758,23 @@ static void qmp_ufs_init_registers(struct qmp_ufs *qmp, const struct qmp_phy_cfg
 		qmp_ufs_init_all(qmp, &cfg->tbls_hs_b);
 }

-static int qmp_ufs_com_init(struct qmp_ufs *qmp)
+static int qmp_ufs_com_exit(struct qmp_ufs *qmp)
 {
 	const struct qmp_phy_cfg *cfg = qmp->cfg;
+
+	reset_control_assert(qmp->ufs_reset);
+
+	clk_bulk_disable_unprepare(qmp->num_clks, qmp->clks);
+
+	regulator_bulk_disable(cfg->num_vregs, qmp->vregs);
+
+	return 0;
+}
+
+static int qmp_ufs_power_on(struct phy *phy)
+{
+	struct qmp_ufs *qmp = phy_get_drvdata(phy);
+	const struct qmp_phy_cfg *cfg = qmp->cfg;
 	void __iomem *pcs = qmp->pcs;
 	int ret;

@@ -1775,35 +1789,10 @@ static int qmp_ufs_com_init(struct qmp_ufs *qmp)
 		goto err_disable_regulators;

 	qphy_setbits(pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL], SW_PWRDN);
-
 	return 0;

 err_disable_regulators:
 	regulator_bulk_disable(cfg->num_vregs, qmp->vregs);
-
-	return ret;
-}
-
-static int qmp_ufs_com_exit(struct qmp_ufs *qmp)
-{
-	const struct qmp_phy_cfg *cfg = qmp->cfg;
-
-	reset_control_assert(qmp->ufs_reset);
-
-	clk_bulk_disable_unprepare(qmp->num_clks, qmp->clks);
-
-	regulator_bulk_disable(cfg->num_vregs, qmp->vregs);
-
-	return 0;
-}
-
-static int qmp_ufs_power_on(struct phy *phy)
-{
-	struct qmp_ufs *qmp = phy_get_drvdata(phy);
-	int ret;
-	dev_vdbg(qmp->dev, "Initializing QMP phy\n");
-
-	ret = qmp_ufs_com_init(qmp);
 	return ret;
 }

--
2.48.1

[PATCH V6 07/10] phy: qcom-qmp-ufs: Rename qmp_ufs_power_off

[Nitin Rawat]

Rename qmp_ufs_disable to qmp_ufs_power_off to better represent its
functionality. Additionally, inline qmp_ufs_exit into qmp_ufs_power_off
function to preserve the functionality of .power_off.

There is no functional change.

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 19 +------------------
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
index eda0a59918ea..e0dc5fa43dee 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
@@ -1849,28 +1849,11 @@ static int qmp_ufs_power_off(struct phy *phy)
 	qphy_clrbits(qmp->pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL],
 			SW_PWRDN);

-	return 0;
-}
-
-static int qmp_ufs_exit(struct phy *phy)
-{
-	struct qmp_ufs *qmp = phy_get_drvdata(phy);
-
 	qmp_ufs_com_exit(qmp);

 	return 0;
 }

-static int qmp_ufs_disable(struct phy *phy)
-{
-	int ret;
-
-	ret = qmp_ufs_power_off(phy);
-	if (ret)
-		return ret;
-	return qmp_ufs_exit(phy);
-}
-
 static int qmp_ufs_set_mode(struct phy *phy, enum phy_mode mode, int submode)
 {
 	struct qmp_ufs *qmp = phy_get_drvdata(phy);
@@ -1919,7 +1902,7 @@ static int qmp_ufs_phy_init(struct phy *phy)
 static const struct phy_ops qcom_qmp_ufs_phy_ops = {
 	.init		= qmp_ufs_phy_init,
 	.power_on	= qmp_ufs_power_on,
-	.power_off	= qmp_ufs_disable,
+	.power_off	= qmp_ufs_power_off,
 	.calibrate	= qmp_ufs_phy_calibrate,
 	.set_mode	= qmp_ufs_set_mode,
 	.owner		= THIS_MODULE,
--
2.48.1

[PATCH V6 08/10] phy: qcom-qmp-ufs: Remove qmp_ufs_exit() and Inline qmp_ufs_com_exit()

[Nitin Rawat]

qmp_ufs_exit() is a wrapper function. It only calls qmp_ufs_com_exit().
Remove it to simplify the ufs phy driver.

Additonally partial Inline(dropping the reset assert) qmp_ufs_com_exit
into qmp_ufs_power_off function to avoid unnecessary function call
and to align with the Phy programming guide.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
index e0dc5fa43dee..00bde65733cb 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
@@ -1758,19 +1758,6 @@ static void qmp_ufs_init_registers(struct qmp_ufs *qmp, const struct qmp_phy_cfg
 		qmp_ufs_init_all(qmp, &cfg->tbls_hs_b);
 }

-static int qmp_ufs_com_exit(struct qmp_ufs *qmp)
-{
-	const struct qmp_phy_cfg *cfg = qmp->cfg;
-
-	reset_control_assert(qmp->ufs_reset);
-
-	clk_bulk_disable_unprepare(qmp->num_clks, qmp->clks);
-
-	regulator_bulk_disable(cfg->num_vregs, qmp->vregs);
-
-	return 0;
-}
-
 static int qmp_ufs_power_on(struct phy *phy)
 {
 	struct qmp_ufs *qmp = phy_get_drvdata(phy);
@@ -1849,7 +1836,9 @@ static int qmp_ufs_power_off(struct phy *phy)
 	qphy_clrbits(qmp->pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL],
 			SW_PWRDN);

-	qmp_ufs_com_exit(qmp);
+	clk_bulk_disable_unprepare(qmp->num_clks, qmp->clks);
+
+	regulator_bulk_disable(cfg->num_vregs, qmp->vregs);

 	return 0;
 }
--
2.48.1

[PATCH V6 09/10] phy: qcom-qmp-ufs: refactor qmp_ufs_power_off

[Nitin Rawat]

In qmp_ufs_power_off, the PHY is already powered down by asserting
QPHY_PCS_POWER_DOWN_CONTROL. Therefore, additional phy_reset and
stopping SerDes are unnecessary. Also this approach does not
align with the phy HW programming guide.

Thus, refactor qmp_ufs_power_off to remove the phy_reset and stop
SerDes calls to simplify the code and ensure alignment with the PHY
HW programming guide.

Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
index 00bde65733cb..9c69c77d10c8 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
@@ -1825,13 +1825,6 @@ static int qmp_ufs_power_off(struct phy *phy)
 	struct qmp_ufs *qmp = phy_get_drvdata(phy);
 	const struct qmp_phy_cfg *cfg = qmp->cfg;

-	/* PHY reset */
-	if (!cfg->no_pcs_sw_reset)
-		qphy_setbits(qmp->pcs, cfg->regs[QPHY_SW_RESET], SW_RESET);
-
-	/* stop SerDes */
-	qphy_clrbits(qmp->pcs, cfg->regs[QPHY_START_CTRL], SERDES_START);
-
 	/* Put PHY into POWER DOWN state: active low */
 	qphy_clrbits(qmp->pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL],
 			SW_PWRDN);
--
2.48.1

[PATCH V6 10/10] scsi: ufs: qcom : Refactor phy_power_on/off calls

[Nitin Rawat]

Commit 3f6d1767b1a0 ("phy: ufs-qcom: Refactor all init steps into
phy_poweron") moved the phy_power_on/off from ufs_qcom_setup_clocks
to suspend/resume func.

To have a better power saving, remove the phy_power_on/off calls from
resume/suspend path and put them back to ufs_qcom_setup_clocks, so that
PHY regulators & clks can be turned on/off along with UFS's clocks.

Since phy phy_power_on is separated out from phy calibrate, make
separate calls to phy_power_on calls from ufs qcom driver.

Co-developed-by: Can Guo <quic_cang@quicinc.com>
Signed-off-by: Can Guo <quic_cang@quicinc.com>
Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
---
 drivers/ufs/host/ufs-qcom.c | 58 +++++++++++++++++--------------------
 1 file changed, 27 insertions(+), 31 deletions(-)

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index dfe164da3668..1b313c1c530f 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -696,26 +696,17 @@ static int ufs_qcom_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op,
 	enum ufs_notify_change_status status)
 {
 	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
-	struct phy *phy = host->generic_phy;

 	if (status == PRE_CHANGE)
 		return 0;

-	if (ufs_qcom_is_link_off(hba)) {
-		/*
-		 * Disable the tx/rx lane symbol clocks before PHY is
-		 * powered down as the PLL source should be disabled
-		 * after downstream clocks are disabled.
-		 */
+	if (!ufs_qcom_is_link_active(hba))
 		ufs_qcom_disable_lane_clks(host);
-		phy_power_off(phy);

-		/* reset the connected UFS device during power down */
-		ufs_qcom_device_reset_ctrl(hba, true);

-	} else if (!ufs_qcom_is_link_active(hba)) {
-		ufs_qcom_disable_lane_clks(host);
-	}
+	/* reset the connected UFS device during power down */
+	if (ufs_qcom_is_link_off(hba) && host->device_reset)
+		ufs_qcom_device_reset_ctrl(hba, true);

 	return ufs_qcom_ice_suspend(host);
 }
@@ -723,26 +714,11 @@ static int ufs_qcom_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op,
 static int ufs_qcom_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 {
 	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
-	struct phy *phy = host->generic_phy;
 	int err;

-	if (ufs_qcom_is_link_off(hba)) {
-		err = phy_power_on(phy);
-		if (err) {
-			dev_err(hba->dev, "%s: failed PHY power on: %d\n",
-				__func__, err);
-			return err;
-		}
-
-		err = ufs_qcom_enable_lane_clks(host);
-		if (err)
-			return err;
-
-	} else if (!ufs_qcom_is_link_active(hba)) {
-		err = ufs_qcom_enable_lane_clks(host);
-		if (err)
-			return err;
-	}
+	err = ufs_qcom_enable_lane_clks(host);
+	if (err)
+		return err;

 	return ufs_qcom_ice_resume(host);
 }
@@ -1132,12 +1108,20 @@ static void ufs_qcom_set_caps(struct ufs_hba *hba)
  * @on: If true, enable clocks else disable them.
  * @status: PRE_CHANGE or POST_CHANGE notify
  *
+ * There are certain clocks which comes from the PHY so it needs
+ * to be managed together along with controller clocks which also
+ * provides a better power saving. Hence keep phy_power_off/on calls
+ * in ufs_qcom_setup_clocks, so that PHY's regulators & clks can be
+ * turned on/off along with UFS's clocks.
+ *
  * Return: 0 on success, non-zero on failure.
  */
 static int ufs_qcom_setup_clocks(struct ufs_hba *hba, bool on,
 				 enum ufs_notify_change_status status)
 {
 	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+	struct phy *phy = host->generic_phy;
+	int err;

 	/*
 	 * In case ufs_qcom_init() is not yet done, simply ignore.
@@ -1156,10 +1140,22 @@ static int ufs_qcom_setup_clocks(struct ufs_hba *hba, bool on,
 				/* disable device ref_clk */
 				ufs_qcom_dev_ref_clk_ctrl(host, false);
 			}
+
+			err = phy_power_off(phy);
+			if (err) {
+				dev_err(hba->dev, "phy power off failed, ret=%d\n", err);
+				return err;
+			}
 		}
 		break;
 	case POST_CHANGE:
 		if (on) {
+			err = phy_power_on(phy);
+			if (err) {
+				dev_err(hba->dev, "phy power on failed, ret = %d\n", err);
+				return err;
+			}
+
 			/* enable the device ref clock for HS mode*/
 			if (ufshcd_is_hs_mode(&hba->pwr_info))
 				ufs_qcom_dev_ref_clk_ctrl(host, true);
--
2.48.1

Re: [PATCH V6 00/10] Refactor ufs phy powerup sequence

[Martin K. Petersen]

Nitin,

>       Patch 1 is a fix for existing issues, which doesn't have any
>       dependencies on any other changes.

Applied patch 1 to 6.16/scsi-staging, thanks!

-- 
Martin K. Petersen

Re: [PATCH V6 00/10] Refactor ufs phy powerup sequence

[Nitin Rawat]

On 5/28/2025 7:45 AM, Martin K. Petersen wrote:
> 
> Nitin,
> 
>>        Patch 1 is a fix for existing issues, which doesn't have any
>>        dependencies on any other changes.

Hi Martin and Vinod,

For the remaining patches (Patch 2 - Patch 10), since Patch 2 (SCSI 
Patch) depends on PHY patches, can these patches be merged through the 
PHY tree?

Regards,
Nitin




> 
> Applied patch 1 to 6.16/scsi-staging, thanks!
> 

Re: [PATCH V6 00/10] Refactor ufs phy powerup sequence

[Martin K. Petersen]

Nitin,

> For the remaining patches (Patch 2 - Patch 10), since Patch 2 (SCSI
> Patch) depends on PHY patches, can these patches be merged through the
> PHY tree?

Fine with me.

Acked-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen

Re: (subset) [PATCH V6 00/10] Refactor ufs phy powerup sequence

[Vinod Koul]

On Mon, 26 May 2025 21:08:11 +0530, Nitin Rawat wrote:
> In Current code regulators enable, clks enable, calibrating UFS PHY,
> start_serdes and polling PCS_ready_status are part of phy_power_on.
> 
> UFS PHY registers are retained after power collapse, meaning calibrating
> UFS PHY, start_serdes and polling PCS_ready_status can be done only when
> hba is powered_on, and not needed every time when phy_power_on is called
> during resume. Hence keep the code which enables PHY's regulators & clks
> in phy_power_on and move the rest steps into phy_calibrate function.
> 
> [...]

Applied, thanks!

[02/10] scsi: ufs: qcom: add a new phy calibrate API call
        commit: 399c75b6a9ed2fd609f9ad4c22cdd6364bc9d441
[03/10] phy: qcom-qmp-ufs: Rename qmp_ufs_enable and qmp_ufs_power_on
        commit: dbd20821946a74e803208a25dddfafe1ae2e34e6
[04/10] phy: qcom-qmp-ufs: Refactor phy_power_on and phy_calibrate callbacks
        commit: cbfd6c124f27ad2b4c0f617dc40ad8a08a063463
[05/10] phy: qcom-qmp-ufs: Refactor UFS PHY reset
        commit: d58b9ff47775042acc501d0a892af8bd08128a65
[06/10] phy: qcom-qmp-ufs: Remove qmp_ufs_com_init()
        commit: 7bcf4936aac6ec8d6fafbfd6f4f62302e5296a0d
[07/10] phy: qcom-qmp-ufs: Rename qmp_ufs_power_off
        commit: acc6b0d73d902d3296d8c77878a9b508c2c6a5bf
[08/10] phy: qcom-qmp-ufs: Remove qmp_ufs_exit() and Inline qmp_ufs_com_exit()
        commit: 7f600f0e193a6638135026c3718ac296ed3f5044
[09/10] phy: qcom-qmp-ufs: refactor qmp_ufs_power_off
        commit: a079b2d715340482e425ff136b55810ab8279800
[10/10] scsi: ufs: qcom : Refactor phy_power_on/off calls
        commit: 77d2fa54a94574f767d5fb296b6b8e011eba0c8e

Best regards,
-- 
~Vinod

Re: [PATCH V6 10/10] scsi: ufs: qcom : Refactor phy_power_on/off calls

[Aishwarya]

Hi Nitin,

When booting the kernel from next-20250619 on Arm64 Qualcomm boards
(RB5 and DB845C), we observe that the baseline bootr test fails due to
dmesg.emerg errors in our CI environment.

A full git bisect (log included below) points to this patch as the
culprit. The issue was introduced sometime from tag next-20250616 in
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git.

This issue is not present in v6.16-rc2

Here’s a sample of the failure observed on the RB5 board:
lert :   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
kern  :alert :   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
kern  :alert :   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
kern  :alert : user pgtable: 4k pages, 48-bit VAs, pgdp=0000000102c4c000
kern  :alert : [0000000000000000] pgd=0000000000000000
kern  :emerg : Internal error: Oops: 0000000096000004 [#1]  SMP
kern  :emerg : Code: a90157f3 aa0003f3 f90013f6 f9405c15 (f94002b6) 
<8>[   30.933289] <LAVA_SIGNAL_TESTCASE TEST_CASE_ID=emerg RESULT=fail UNITS=lines MEASUREMENT=2>
+ <8>[   30.943798] <LAVA_SIGNAL_ENDRUN 0_dmesg 1000325_2.4.4.1>
set +x

Git bisection log:
git bisect start
# status: waiting for both good and bad commits
# good: [9afe652958c3ee88f24df1e4a97f298afce89407] Merge tag 'x86_urgent_for_6.16-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect good 9afe652958c3ee88f24df1e4a97f298afce89407
# status: waiting for bad commit, 1 good commit known
# bad: [4325743c7e209ae7845293679a4de94b969f2bef] Add linux-next specific files for 20250617
git bisect bad 4325743c7e209ae7845293679a4de94b969f2bef
# good: [436c8cbbcb18deb96100cd9f33f1efedddc31d9c] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git
git bisect good 436c8cbbcb18deb96100cd9f33f1efedddc31d9c
# good: [183d224083773ca4a84a458fb608efecff19e19e] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git
git bisect good 183d224083773ca4a84a458fb608efecff19e19e
# good: [1347ff0c0e510f1a6adb78259a2a0ddfac41d258] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git
git bisect good 1347ff0c0e510f1a6adb78259a2a0ddfac41d258
# bad: [b09bd04eabb1994257f4c11d0ed25ff03517d3ec] Merge branch 'gpio/for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git
git bisect bad b09bd04eabb1994257f4c11d0ed25ff03517d3ec
# good: [70bc9e18653c20fbcb47184d9498ad7bd7b7d6be] Merge branch 'togreg' of git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git
git bisect good 70bc9e18653c20fbcb47184d9498ad7bd7b7d6be
# bad: [3a847fb85c9d47e61ad5d9d54b762a3e99a08084] Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/phy/linux-phy.git
git bisect bad 3a847fb85c9d47e61ad5d9d54b762a3e99a08084
# skip: [50355ac70d4f104e2f82bfbd0658c129027ebb37] dt-bindings: phy: Convert marvell,comphy-cp110 to DT schema
git bisect skip 50355ac70d4f104e2f82bfbd0658c129027ebb37
# good: [acc6b0d73d902d3296d8c77878a9b508c2c6a5bf] phy: qcom-qmp-ufs: Rename qmp_ufs_power_off
git bisect good acc6b0d73d902d3296d8c77878a9b508c2c6a5bf
# bad: [35b629b28afd72a14ed573f1b180dc4ab1bf7e19] dt-bindings: phy: Convert ti,dm816x-usb-phy to DT schema
git bisect bad 35b629b28afd72a14ed573f1b180dc4ab1bf7e19
# bad: [66acaf8f6b0bcc273f8356b2a77baa90b177014c] dt-bindings: phy: Convert img,pistachio-usb-phy to DT schema
git bisect bad 66acaf8f6b0bcc273f8356b2a77baa90b177014c
# bad: [f151f3a6ebe184b5f8c9abe58fe2d63f9950139b] dt-bindings: phy: Convert brcm,ns2-drd-phy to DT schema
git bisect bad f151f3a6ebe184b5f8c9abe58fe2d63f9950139b
# bad: [77d2fa54a94574f767d5fb296b6b8e011eba0c8e] scsi: ufs: qcom : Refactor phy_power_on/off calls
git bisect bad 77d2fa54a94574f767d5fb296b6b8e011eba0c8e
# good: [7f600f0e193a6638135026c3718ac296ed3f5044] phy: qcom-qmp-ufs: Remove qmp_ufs_exit() and Inline qmp_ufs_com_exit()
git bisect good 7f600f0e193a6638135026c3718ac296ed3f5044
# good: [a079b2d715340482e425ff136b55810ab8279800] phy: qcom-qmp-ufs: refactor qmp_ufs_power_off
git bisect good a079b2d715340482e425ff136b55810ab8279800
# first bad commit: [77d2fa54a94574f767d5fb296b6b8e011eba0c8e] scsi: ufs: qcom : Refactor phy_power_on/off calls

Thanks,
Aishwarya

Re: [PATCH V6 10/10] scsi: ufs: qcom : Refactor phy_power_on/off calls

[Nitin Rawat]

On 6/19/2025 7:23 PM, Aishwarya wrote:
> Hi Nitin,
> 
> When booting the kernel from next-20250619 on Arm64 Qualcomm boards
> (RB5 and DB845C), we observe that the baseline bootr test fails due to
> dmesg.emerg errors in our CI environment.
> 
> A full git bisect (log included below) points to this patch as the
> culprit. The issue was introduced sometime from tag next-20250616 in
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git.


Hi Aishwarya,

I tested booting the QRB5165 using the qrb5165-rb5.dtb with the latest 
upstream tip multiple times, and each time it successfully booted to the 
shell without any issues.

Are you encountering a boot failure, or is there a specific test case 
you're running that triggers the problem? If so, could you please share 
the dmesg log to help check further.

Thanks,
Nitin

> 
> This issue is not present in v6.16-rc2
> 
> Here’s a sample of the failure observed on the RB5 board:
> lert :   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
> kern  :alert :   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> kern  :alert :   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> kern  :alert : user pgtable: 4k pages, 48-bit VAs, pgdp=0000000102c4c000
> kern  :alert : [0000000000000000] pgd=0000000000000000
> kern  :emerg : Internal error: Oops: 0000000096000004 [#1]  SMP
> kern  :emerg : Code: a90157f3 aa0003f3 f90013f6 f9405c15 (f94002b6)
> <8>[   30.933289] <LAVA_SIGNAL_TESTCASE TEST_CASE_ID=emerg RESULT=fail UNITS=lines MEASUREMENT=2>
> + <8>[   30.943798] <LAVA_SIGNAL_ENDRUN 0_dmesg 1000325_2.4.4.1>
> set +x
> 
> Git bisection log:
> git bisect start
> # status: waiting for both good and bad commits
> # good: [9afe652958c3ee88f24df1e4a97f298afce89407] Merge tag 'x86_urgent_for_6.16-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
> git bisect good 9afe652958c3ee88f24df1e4a97f298afce89407
> # status: waiting for bad commit, 1 good commit known
> # bad: [4325743c7e209ae7845293679a4de94b969f2bef] Add linux-next specific files for 20250617
> git bisect bad 4325743c7e209ae7845293679a4de94b969f2bef
> # good: [436c8cbbcb18deb96100cd9f33f1efedddc31d9c] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git
> git bisect good 436c8cbbcb18deb96100cd9f33f1efedddc31d9c
> # good: [183d224083773ca4a84a458fb608efecff19e19e] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git
> git bisect good 183d224083773ca4a84a458fb608efecff19e19e
> # good: [1347ff0c0e510f1a6adb78259a2a0ddfac41d258] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git
> git bisect good 1347ff0c0e510f1a6adb78259a2a0ddfac41d258
> # bad: [b09bd04eabb1994257f4c11d0ed25ff03517d3ec] Merge branch 'gpio/for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git
> git bisect bad b09bd04eabb1994257f4c11d0ed25ff03517d3ec
> # good: [70bc9e18653c20fbcb47184d9498ad7bd7b7d6be] Merge branch 'togreg' of git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git
> git bisect good 70bc9e18653c20fbcb47184d9498ad7bd7b7d6be
> # bad: [3a847fb85c9d47e61ad5d9d54b762a3e99a08084] Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/phy/linux-phy.git
> git bisect bad 3a847fb85c9d47e61ad5d9d54b762a3e99a08084
> # skip: [50355ac70d4f104e2f82bfbd0658c129027ebb37] dt-bindings: phy: Convert marvell,comphy-cp110 to DT schema
> git bisect skip 50355ac70d4f104e2f82bfbd0658c129027ebb37
> # good: [acc6b0d73d902d3296d8c77878a9b508c2c6a5bf] phy: qcom-qmp-ufs: Rename qmp_ufs_power_off
> git bisect good acc6b0d73d902d3296d8c77878a9b508c2c6a5bf
> # bad: [35b629b28afd72a14ed573f1b180dc4ab1bf7e19] dt-bindings: phy: Convert ti,dm816x-usb-phy to DT schema
> git bisect bad 35b629b28afd72a14ed573f1b180dc4ab1bf7e19
> # bad: [66acaf8f6b0bcc273f8356b2a77baa90b177014c] dt-bindings: phy: Convert img,pistachio-usb-phy to DT schema
> git bisect bad 66acaf8f6b0bcc273f8356b2a77baa90b177014c
> # bad: [f151f3a6ebe184b5f8c9abe58fe2d63f9950139b] dt-bindings: phy: Convert brcm,ns2-drd-phy to DT schema
> git bisect bad f151f3a6ebe184b5f8c9abe58fe2d63f9950139b
> # bad: [77d2fa54a94574f767d5fb296b6b8e011eba0c8e] scsi: ufs: qcom : Refactor phy_power_on/off calls
> git bisect bad 77d2fa54a94574f767d5fb296b6b8e011eba0c8e
> # good: [7f600f0e193a6638135026c3718ac296ed3f5044] phy: qcom-qmp-ufs: Remove qmp_ufs_exit() and Inline qmp_ufs_com_exit()
> git bisect good 7f600f0e193a6638135026c3718ac296ed3f5044
> # good: [a079b2d715340482e425ff136b55810ab8279800] phy: qcom-qmp-ufs: refactor qmp_ufs_power_off
> git bisect good a079b2d715340482e425ff136b55810ab8279800
> # first bad commit: [77d2fa54a94574f767d5fb296b6b8e011eba0c8e] scsi: ufs: qcom : Refactor phy_power_on/off calls
> 
> Thanks,
> Aishwarya

Re: [PATCH V6 10/10] scsi: ufs: qcom : Refactor phy_power_on/off calls

[Aishwarya]

Hi Nitin,

To clarify — the defconfig kernel does boot successfully on our Arm64
Qualcomm platforms (RB5 and DB845C). However, starting from
next-20250613, we are seeing the following three test failures in the
`bootrr` baseline test in our CI environment:

  - baseline.bootrr.scsi-disk-device0-probed
  - dmesg.alert
  - dmesg.emerg

Test suite:
  https://github.com/kernelci/bootrr/tree/main

These failures are due to kernel alerts seen in the boot logs. A relevant
snippet is shown below:

  kern  :alert : Unable to handle kernel NULL pointer dereference at
  virtual address 0000000000000000
  kern  :alert : Mem abort info:
  kern  :alert :   ESR = 0x0000000096000004
  kern  :alert :   EC = 0x25: DABT (current EL), IL = 32 bits
  kern  :alert :   SET = 0, FnV = 0
  kern  :alert :   EA = 0, S1PTW = 0
  kern  :alert :   FSC = 0x04: level 0 translation fault
  kern  :alert : Data abort info:
  kern  :alert :   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
  kern  :alert :   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
  kern  :alert :   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
  kern  :alert : user pgtable: 4k pages, 48-bit VAs, pgdp=0000000109c41000
  kern  :alert : [0000000000000000] pgd=0000000000000000
  <LAVA_SIGNAL_TESTCASE TEST_CASE_ID=alert RESULT=fail UNITS=lines
  MEASUREMENT=13>

  kern  :emerg : Internal error: Oops: 0000000096000004 [#1] SMP
  kern  :emerg : Code: a90157f3 aa0003f3 f90013f6 f9405c15 (f94002b6)
  <LAVA_SIGNAL_TESTCASE TEST_CASE_ID=emerg RESULT=fail UNITS=lines
  MEASUREMENT=2>

Please let me know if you need full logs or further details to help
with debugging.

Thanks,
Aishwarya

Re: [PATCH V6 10/10] scsi: ufs: qcom : Refactor phy_power_on/off calls

[Nitin Rawat]

[-- Attachment #1: Type: text/plain, Size: 1932 bytes --]



On 6/21/2025 3:14 AM, Aishwarya wrote:
> Hi Nitin,
> 
> To clarify — the defconfig kernel does boot successfully on our Arm64
> Qualcomm platforms (RB5 and DB845C). However, starting from
> next-20250613, we are seeing the following three test failures in the
> `bootrr` baseline test in our CI environment:
> 
>    - baseline.bootrr.scsi-disk-device0-probed
>    - dmesg.alert
>    - dmesg.emerg
> 

Hi Aishwarya,

Thanks for testing and reporting this issue. Can you please
test with the attached fix and let me know if it helps.

Regards,
Nitin

> Test suite:
>    https://github.com/kernelci/bootrr/tree/main
> 
> These failures are due to kernel alerts seen in the boot logs. A relevant
> snippet is shown below:
> 
>    kern  :alert : Unable to handle kernel NULL pointer dereference at
>    virtual address 0000000000000000
>    kern  :alert : Mem abort info:
>    kern  :alert :   ESR = 0x0000000096000004
>    kern  :alert :   EC = 0x25: DABT (current EL), IL = 32 bits
>    kern  :alert :   SET = 0, FnV = 0
>    kern  :alert :   EA = 0, S1PTW = 0
>    kern  :alert :   FSC = 0x04: level 0 translation fault
>    kern  :alert : Data abort info:
>    kern  :alert :   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
>    kern  :alert :   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
>    kern  :alert :   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
>    kern  :alert : user pgtable: 4k pages, 48-bit VAs, pgdp=0000000109c41000
>    kern  :alert : [0000000000000000] pgd=0000000000000000
>    <LAVA_SIGNAL_TESTCASE TEST_CASE_ID=alert RESULT=fail UNITS=lines
>    MEASUREMENT=13>
> 
>    kern  :emerg : Internal error: Oops: 0000000096000004 [#1] SMP
>    kern  :emerg : Code: a90157f3 aa0003f3 f90013f6 f9405c15 (f94002b6)
>    <LAVA_SIGNAL_TESTCASE TEST_CASE_ID=emerg RESULT=fail UNITS=lines
>    MEASUREMENT=2>
> 
> Please let me know if you need full logs or further details to help
> with debugging.
> 
> Thanks,
> Aishwarya

[-- Attachment #2: 0001-scsi-ufs-qcom-Fix-NULL-pointer-dereference-in-ufs_qc.patch --]
[-- Type: text/plain, Size: 3029 bytes --]

From 3f6abf0f5a1ad6dba975824c97c94a77babb9d38 Mon Sep 17 00:00:00 2001
From: Nitin Rawat <quic_nitirawa@quicinc.com>
Date: Sat, 21 Jun 2025 21:40:42 +0530
Subject: [PATCH V1] scsi: ufs: qcom : Fix NULL pointer dereference in
 ufs_qcom_setup_clocks

Fix a NULL pointer dereference in ufs_qcom_setup_clocks due to an
uninitialized 'host' variable. The variable 'phy' is now assigned
after confirming 'host' is not NULL.

Call Stack:

[    6.448070] Unable to handle kernel NULL pointer dereference at
virtual address 0000000000000000
[    6.448449] ufs_qcom_setup_clocks+0x28/0x148 ufs_qcom (P)
[    6.448466] ufshcd_setup_clocks (drivers/ufs/core/ufshcd-priv.h:142)
[    6.448477] ufshcd_init (drivers/ufs/core/ufshcd.c:9468)
[    6.448485] ufshcd_pltfrm_init (drivers/ufs/host/ufshcd-pltfrm.c:504)
[    6.448495] ufs_qcom_probe+0x28/0x68 ufs_qcom
[    6.448508] platform_probe (drivers/base/platform.c:1404)
[    6.448519] really_probe (drivers/base/dd.c:579 drivers/base/dd.c:657)
[    6.448526] __driver_probe_device (drivers/base/dd.c:799)
[    6.448532] driver_probe_device (drivers/base/dd.c:829)
[    6.448539] __driver_attach (drivers/base/dd.c:1216)
[    6.448545] bus_for_each_dev (drivers/base/bus.c:370)
[    6.448556] driver_attach (drivers/base/dd.c:1234)
[    6.448567] bus_add_driver (drivers/base/bus.c:678)
[    6.448577] driver_register (drivers/base/driver.c:249)
[    6.448584] __platform_driver_register (drivers/base/platform.c:868)
[    6.448592] ufs_qcom_pltform_init+0x28/0xff8 ufs_qcom
[    6.448605] do_one_initcall (init/main.c:1274)
[    6.448615] do_init_module (kernel/module/main.c:3041)
[    6.448626] load_module (kernel/module/main.c:3511)
[    6.448635] init_module_from_file (kernel/module/main.c:3704)
[    6.448644] __arm64_sys_finit_module (kernel/module/main.c:3715.

Fixes: 77d2fa54a945 ("scsi: ufs: qcom : Refactor phy_power_on/off calls")
Reported-by: Aishwarya <aishwarya.tcv@arm.com>
Closes: https://lore.kernel.org/lkml/20250620214408.11028-1-aishwarya.tcv@arm.com/
Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Closes: https://lkml.org/lkml/2025/6/21/107
Co-developed-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
---
 drivers/ufs/host/ufs-qcom.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index ba4b2880279c..318dca7fe3d7 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -1124,7 +1124,7 @@ static int ufs_qcom_setup_clocks(struct ufs_hba *hba, bool on,
 				 enum ufs_notify_change_status status)
 {
 	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
-	struct phy *phy = host->generic_phy;
+	struct phy *phy;
 	int err;

 	/*
@@ -1135,6 +1135,8 @@ static int ufs_qcom_setup_clocks(struct ufs_hba *hba, bool on,
 	if (!host)
 		return 0;

+	phy = host->generic_phy;
+
 	switch (status) {
 	case PRE_CHANGE:
 		if (on) {
--
2.48.1

Re: [PATCH V6 01/10] scsi: ufs: qcom: Prevent calling phy_exit before phy_init

[Abel Vesa]

On 25-05-26 21:08:12, Nitin Rawat wrote:
> Prevent calling phy_exit before phy_init to avoid abnormal power
> count and the following warning during boot up.
> 
> [5.146763] phy phy-1d80000.phy.0: phy_power_on was called before phy_init
> 
> Fixes: 7bac65687510 ("scsi: ufs: qcom: Power off the PHY if it was already powered on in ufs_qcom_power_up_sequence()")
> Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Reviewed-by: Abel Vesa <abel.vesa@linaro.org>

Re: [PATCH V6 02/10] scsi: ufs: qcom: add a new phy calibrate API call

[Abel Vesa]

On 25-05-26 21:08:13, Nitin Rawat wrote:
> Introduce a new phy calibrate API call in the UFS Qualcomm driver to
> separate phy calibration from phy power-on. This change is a precursor
> to the successive commits in this series, which requires these two
> operations to be distinct.
> 
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>

Reviewed-by: Abel Vesa <abel.vesa@linaro.org>

Re: [PATCH V6 10/10] scsi: ufs: qcom : Refactor phy_power_on/off calls

[Marek Szyprowski]

On 21.06.2025 19:15, Nitin Rawat wrote:
>
> On 6/21/2025 3:14 AM, Aishwarya wrote:
>> Hi Nitin,
>>
>> To clarify — the defconfig kernel does boot successfully on our Arm64
>> Qualcomm platforms (RB5 and DB845C). However, starting from
>> next-20250613, we are seeing the following three test failures in the
>> `bootrr` baseline test in our CI environment:
>>
>>    - baseline.bootrr.scsi-disk-device0-probed
>>    - dmesg.alert
>>    - dmesg.emerg
>>
>
> Hi Aishwarya,
>
> Thanks for testing and reporting this issue. Can you please
> test with the attached fix and let me know if it helps.
>
I also stepped into this issue on the RB5 board and I confirm that that 
patch fixes it. Thanks!

Feel free to add:

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>


 > ...

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland