2018-07-31 10:10:33

by Can Guo

[permalink] [raw]
Subject: [PATCH v8 0/5] Support for Qualcomm UFS QMP PHY on SDM845

This patch series adds support for UFS QMP PHY on SDM845 and the
compatible string for it. This patch series depends on the current
proposed QMP V3 USB3 UNI PHY support for sdm845 driver [1], on
the DT bindings for the QMP V3 USB3 PHYs based dirver [2], and also
rebased on updated pipe_clk initialization sequence [3]. This series
can only be merged once the dependent patches do.
[1] http://lists-archives.com/linux-kernel/29071659-dt-bindings-phy-qcom-qmp-update-bindings-for-sdm845.html
[2] http://lists-archives.com/linux-kernel/29071660-phy-qcom-qmp-add-qmp-v3-usb3-uni-phy-support-for-sdm845.html
[3] https://patchwork.kernel.org/patch/10376551/

Changes since v7:
- Add one new change to update UFS PHY power on sequence
- Incorporated review comments from Evan, Vivek and Manu.

Changes since v6:
- Add one new change to clean up some structs and field
- Updates the PHY power control sequence.
- Incorporated review comments from Vivek and Manu.

Changes since v5:
- Updates the PHY power control sequence.
- Updates UFS PHY power on condition check.

Changes since v4:
- Adds 'ref_aux' clock back to SDM845 UFS PHY clock list.
- Power on PHY before serdes configuration starts.
- Updates the UFS PHY initialization sequence.
- Updates a few UFS PHY registers.
- Incorporated review comments from Vivek and Manu.

Changes since v3:
- Incorporated review comments from Vivek and Rob.

Changes since v2:
- Incorporated review comments from Vivek and Rob.
- Remove "ref_aux" from sdm845 ufs phy clock list structure.

Changes since v1:
- Incorporated review comments from Vivek and Manu.
- Update the commit title of patch 2.


Can Guo (5):
phy: Update PHY power control sequence
phy: General struct and field cleanup
phy: Add QMP phy based UFS phy support for sdm845
scsi: ufs: Power on phy after it is initialized
dt-bindings: phy-qcom-qmp: Add UFS phy compatible string for sdm845

.../devicetree/bindings/phy/qcom-qmp-phy.txt | 4 +-
drivers/phy/qualcomm/phy-qcom-qmp.c | 216 +++++++++++++++++++--
drivers/phy/qualcomm/phy-qcom-qmp.h | 15 ++
drivers/scsi/ufs/ufs-qcom.c | 4 +-
drivers/scsi/ufs/ufs-qcom.h | 1 +
5 files changed, 219 insertions(+), 21 deletions(-)

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



2018-07-31 10:10:38

by Can Guo

[permalink] [raw]
Subject: [PATCH v8 1/5] phy: Update PHY power control sequence

All PHYs should be powered on before register configuration starts. And
only PCIe PHYs need an extra power control before deasserts reset state.

Signed-off-by: Can Guo <[email protected]>
Reviewed-by: Manu Gautam <[email protected]>
Reviewed-by: Vivek Gautam <[email protected]>
Reviewed-by: Evan Green <[email protected]>
---
drivers/phy/qualcomm/phy-qcom-qmp.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c
index 97ef942..ccb8578 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
@@ -935,10 +935,12 @@ static void qcom_qmp_phy_configure(void __iomem *base,
}
}

-static int qcom_qmp_phy_com_init(struct qcom_qmp *qmp)
+static int qcom_qmp_phy_com_init(struct qmp_phy *qphy)
{
+ struct qcom_qmp *qmp = qphy->qmp;
const struct qmp_phy_cfg *cfg = qmp->cfg;
void __iomem *serdes = qmp->serdes;
+ void __iomem *pcs = qphy->pcs;
void __iomem *dp_com = qmp->dp_com;
int ret, i;

@@ -979,10 +981,6 @@ static int qcom_qmp_phy_com_init(struct qcom_qmp *qmp)
goto err_rst;
}

- if (cfg->has_phy_com_ctrl)
- qphy_setbits(serdes, cfg->regs[QPHY_COM_POWER_DOWN_CONTROL],
- SW_PWRDN);
-
if (cfg->has_phy_dp_com_ctrl) {
qphy_setbits(dp_com, QPHY_V3_DP_COM_POWER_DOWN_CTRL,
SW_PWRDN);
@@ -1000,6 +998,12 @@ static int qcom_qmp_phy_com_init(struct qcom_qmp *qmp)
SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
}

+ if (cfg->has_phy_com_ctrl)
+ qphy_setbits(serdes, cfg->regs[QPHY_COM_POWER_DOWN_CONTROL],
+ SW_PWRDN);
+ else
+ qphy_setbits(pcs, QPHY_POWER_DOWN_CONTROL, cfg->pwrdn_ctrl);
+
/* Serdes configuration */
qcom_qmp_phy_configure(serdes, cfg->regs, cfg->serdes_tbl,
cfg->serdes_tbl_num);
@@ -1090,7 +1094,7 @@ static int qcom_qmp_phy_init(struct phy *phy)

dev_vdbg(qmp->dev, "Initializing QMP phy\n");

- ret = qcom_qmp_phy_com_init(qmp);
+ ret = qcom_qmp_phy_com_init(qphy);
if (ret)
return ret;

@@ -1127,7 +1131,8 @@ static int qcom_qmp_phy_init(struct phy *phy)
* Pull out PHY from POWER DOWN state.
* This is active low enable signal to power-down PHY.
*/
- qphy_setbits(pcs, QPHY_POWER_DOWN_CONTROL, cfg->pwrdn_ctrl);
+ if(cfg->type == PHY_TYPE_PCIE)
+ qphy_setbits(pcs, QPHY_POWER_DOWN_CONTROL, cfg->pwrdn_ctrl);

if (cfg->has_pwrdn_delay)
usleep_range(cfg->pwrdn_delay_min, cfg->pwrdn_delay_max);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


2018-07-31 10:10:47

by Can Guo

[permalink] [raw]
Subject: [PATCH v8 2/5] phy: General struct and field cleanup

Move MSM8996 specific PHY vreg list struct name to a genernal one as it is
used by all PHYs. Add a specific field to handle dual lane situation.

Signed-off-by: Can Guo <[email protected]>
Reviewed-by: Evan Green <[email protected]>
Reviewed-by: Manu Gautam <[email protected]>
Reviewed-by: Vivek Gautam <[email protected]>
---
drivers/phy/qualcomm/phy-qcom-qmp.c | 25 ++++++++++++++-----------
1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c
index ccb8578..9be9754 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
@@ -649,6 +649,8 @@ struct qmp_phy_cfg {

/* true, if PHY has a separate DP_COM control block */
bool has_phy_dp_com_ctrl;
+ /* true, if PHY has secondary tx/rx lanes to be configured */
+ bool is_dual_lane_phy;
/* Register offset of secondary tx/rx lanes for USB DP combo PHY */
unsigned int tx_b_lane_offset;
unsigned int rx_b_lane_offset;
@@ -758,7 +760,7 @@ static inline void qphy_clrbits(void __iomem *base, u32 offset, u32 val)
};

/* list of regulators */
-static const char * const msm8996_phy_vreg_l[] = {
+static const char * const qmp_phy_vreg_l[] = {
"vdda-phy", "vdda-pll",
};

@@ -778,8 +780,8 @@ static inline void qphy_clrbits(void __iomem *base, u32 offset, u32 val)
.num_clks = ARRAY_SIZE(msm8996_phy_clk_l),
.reset_list = msm8996_pciephy_reset_l,
.num_resets = ARRAY_SIZE(msm8996_pciephy_reset_l),
- .vreg_list = msm8996_phy_vreg_l,
- .num_vregs = ARRAY_SIZE(msm8996_phy_vreg_l),
+ .vreg_list = qmp_phy_vreg_l,
+ .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l),
.regs = pciephy_regs_layout,

.start_ctrl = PCS_START | PLL_READY_GATE_EN,
@@ -809,8 +811,8 @@ static inline void qphy_clrbits(void __iomem *base, u32 offset, u32 val)
.num_clks = ARRAY_SIZE(msm8996_phy_clk_l),
.reset_list = msm8996_usb3phy_reset_l,
.num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
- .vreg_list = msm8996_phy_vreg_l,
- .num_vregs = ARRAY_SIZE(msm8996_phy_vreg_l),
+ .vreg_list = qmp_phy_vreg_l,
+ .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l),
.regs = usb3phy_regs_layout,

.start_ctrl = SERDES_START | PCS_START,
@@ -870,8 +872,8 @@ static inline void qphy_clrbits(void __iomem *base, u32 offset, u32 val)
.num_clks = ARRAY_SIZE(qmp_v3_phy_clk_l),
.reset_list = msm8996_usb3phy_reset_l,
.num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
- .vreg_list = msm8996_phy_vreg_l,
- .num_vregs = ARRAY_SIZE(msm8996_phy_vreg_l),
+ .vreg_list = qmp_phy_vreg_l,
+ .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l),
.regs = qmp_v3_usb3phy_regs_layout,

.start_ctrl = SERDES_START | PCS_START,
@@ -883,6 +885,7 @@ static inline void qphy_clrbits(void __iomem *base, u32 offset, u32 val)
.pwrdn_delay_max = POWER_DOWN_DELAY_US_MAX,

.has_phy_dp_com_ctrl = true,
+ .is_dual_lane_phy = true,
.tx_b_lane_offset = 0x400,
.rx_b_lane_offset = 0x400,
};
@@ -903,8 +906,8 @@ static inline void qphy_clrbits(void __iomem *base, u32 offset, u32 val)
.num_clks = ARRAY_SIZE(qmp_v3_phy_clk_l),
.reset_list = msm8996_usb3phy_reset_l,
.num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
- .vreg_list = msm8996_phy_vreg_l,
- .num_vregs = ARRAY_SIZE(msm8996_phy_vreg_l),
+ .vreg_list = qmp_phy_vreg_l,
+ .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l),
.regs = qmp_v3_usb3phy_regs_layout,

.start_ctrl = SERDES_START | PCS_START,
@@ -1116,12 +1119,12 @@ static int qcom_qmp_phy_init(struct phy *phy)
/* Tx, Rx, and PCS configurations */
qcom_qmp_phy_configure(tx, cfg->regs, cfg->tx_tbl, cfg->tx_tbl_num);
/* Configuration for other LANE for USB-DP combo PHY */
- if (cfg->has_phy_dp_com_ctrl)
+ if (cfg->is_dual_lane_phy)
qcom_qmp_phy_configure(tx + cfg->tx_b_lane_offset, cfg->regs,
cfg->tx_tbl, cfg->tx_tbl_num);

qcom_qmp_phy_configure(rx, cfg->regs, cfg->rx_tbl, cfg->rx_tbl_num);
- if (cfg->has_phy_dp_com_ctrl)
+ if (cfg->is_dual_lane_phy)
qcom_qmp_phy_configure(rx + cfg->rx_b_lane_offset, cfg->regs,
cfg->rx_tbl, cfg->rx_tbl_num);

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


2018-07-31 10:10:53

by Can Guo

[permalink] [raw]
Subject: [PATCH v8 4/5] scsi: ufs: Power on phy after it is initialized

Before UFS PHY is initialized, powering on it has no effect but increases
the PHY's power on count. Then when power on PHY to enable it after phy
initialization where it is really needed, as PHY's power on count is not
zero, again it would do nothing but directly return and leaves the PHY
disabled. This change adds condition checks to phy power on calls which
happen before phy initialization.

Signed-off-by: Can Guo <[email protected]>
---
drivers/scsi/ufs/ufs-qcom.c | 4 +++-
drivers/scsi/ufs/ufs-qcom.h | 1 +
2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 2b38db2..4e9d526 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -288,6 +288,7 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba)
__func__, ret);
goto out;
}
+ host->is_phy_init = true;

/* De-assert PHY reset and start serdes */
ufs_qcom_deassert_reset(hba);
@@ -1147,7 +1148,8 @@ static int ufs_qcom_setup_clocks(struct ufs_hba *hba, bool on,
return 0;

if (on && (status == POST_CHANGE)) {
- phy_power_on(host->generic_phy);
+ if (host->is_phy_init)
+ phy_power_on(host->generic_phy);

/* enable the device ref clock for HS mode*/
if (ufshcd_is_hs_mode(&hba->pwr_info))
diff --git a/drivers/scsi/ufs/ufs-qcom.h b/drivers/scsi/ufs/ufs-qcom.h
index 295f4be..5747aa3 100644
--- a/drivers/scsi/ufs/ufs-qcom.h
+++ b/drivers/scsi/ufs/ufs-qcom.h
@@ -242,6 +242,7 @@ struct ufs_qcom_host {
/* Bitmask for enabling debug prints */
u32 dbg_print_en;
struct ufs_qcom_testbus testbus;
+ bool is_phy_init;
};

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


2018-07-31 10:11:20

by Can Guo

[permalink] [raw]
Subject: [PATCH v8 5/5] dt-bindings: phy-qcom-qmp: Add UFS phy compatible string for sdm845

Update the compatible string for UFS QMP PHY on SDM845.

Signed-off-by: Can Guo <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
index cef8765..930d94c 100644
--- a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
+++ b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
@@ -11,7 +11,8 @@ Required properties:
"qcom,msm8996-qmp-usb3-phy" for 14nm USB3 phy on msm8996,
"qcom,qmp-v3-usb3-phy" for USB3 QMP V3 phy,
"qcom,sdm845-qmp-usb3-phy" for USB3 QMP V3 phy on sdm845,
- "qcom,sdm845-qmp-usb3-uni-phy" for USB3 QMP V3 UNI phy on sdm845.
+ "qcom,sdm845-qmp-usb3-uni-phy" for USB3 QMP V3 UNI phy on sdm845,
+ "qcom,sdm845-qmp-ufs-phy" for UFS QMP phy on sdm845.

- reg: offset and length of register set for PHY's common serdes block.

@@ -29,6 +30,7 @@ Required properties:
"aux" for phy aux clock,
"ref" for 19.2 MHz ref clk,
"com_aux" for phy common block aux clock,
+ "ref_aux" for phy reference aux clock,
For "qcom,msm8996-qmp-pcie-phy" must contain:
"aux", "cfg_ahb", "ref".
For "qcom,msm8996-qmp-usb3-phy" must contain:
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


2018-07-31 10:11:26

by Can Guo

[permalink] [raw]
Subject: [PATCH v8 3/5] phy: Add QMP phy based UFS phy support for sdm845

Add UFS PHY support to make SDM845 UFS work with common PHY framework.

Signed-off-by: Can Guo <[email protected]>
---
drivers/phy/qualcomm/phy-qcom-qmp.c | 172 +++++++++++++++++++++++++++++++++++-
drivers/phy/qualcomm/phy-qcom-qmp.h | 15 ++++
2 files changed, 186 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c
index 9be9754..de7ff18 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
@@ -156,6 +156,11 @@ enum qphy_reg_layout {
[QPHY_PCS_LFPS_RXTERM_IRQ_STATUS] = 0x170,
};

+static const unsigned int sdm845_ufsphy_regs_layout[] = {
+ [QPHY_START_CTRL] = 0x00,
+ [QPHY_PCS_READY_STATUS] = 0x160,
+};
+
static const struct qmp_phy_init_tbl msm8996_pcie_serdes_tbl[] = {
QMP_PHY_INIT_CFG(QSERDES_COM_BIAS_EN_CLKBUFLR_EN, 0x1c),
QMP_PHY_INIT_CFG(QSERDES_COM_CLK_ENABLE1, 0x10),
@@ -601,6 +606,83 @@ enum qphy_reg_layout {
QMP_PHY_INIT_CFG(QPHY_V3_PCS_REFGEN_REQ_CONFIG2, 0x60),
};

+static const struct qmp_phy_init_tbl sdm845_ufsphy_serdes_tbl[] = {
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_SYS_CLK_CTRL, 0x02),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_BIAS_EN_CLKBUFLR_EN, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_BG_TIMER, 0x0a),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_PLL_IVCO, 0x07),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_CMN_CONFIG, 0x06),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_SYSCLK_EN_SEL, 0xd5),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_RESETSM_CNTRL, 0x20),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_CLK_SELECT, 0x30),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_HSCLK_SEL, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_LOCK_CMP_EN, 0x01),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_VCO_TUNE_CTRL, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_CORE_CLK_EN, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_VCO_TUNE_MAP, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_SVS_MODE_CLK_SEL, 0x05),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_VCO_TUNE_INITVAL1, 0xff),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_VCO_TUNE_INITVAL2, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_DEC_START_MODE0, 0x82),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_CP_CTRL_MODE0, 0x06),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_PLL_RCTRL_MODE0, 0x16),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_PLL_CCTRL_MODE0, 0x36),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_INTEGLOOP_GAIN0_MODE0, 0x3f),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_INTEGLOOP_GAIN1_MODE0, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_VCO_TUNE1_MODE0, 0xda),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_VCO_TUNE2_MODE0, 0x01),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_LOCK_CMP1_MODE0, 0xff),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_LOCK_CMP2_MODE0, 0x0c),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_DEC_START_MODE1, 0x98),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_CP_CTRL_MODE1, 0x06),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_PLL_RCTRL_MODE1, 0x16),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_PLL_CCTRL_MODE1, 0x36),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_INTEGLOOP_GAIN0_MODE1, 0x3f),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_INTEGLOOP_GAIN1_MODE1, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_VCO_TUNE1_MODE1, 0xc1),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_VCO_TUNE2_MODE1, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_LOCK_CMP1_MODE1, 0x32),
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_LOCK_CMP2_MODE1, 0x0f),
+
+ /* Rate B */
+ QMP_PHY_INIT_CFG(QSERDES_V3_COM_VCO_TUNE_MAP, 0x44),
+};
+
+static const struct qmp_phy_init_tbl sdm845_ufsphy_tx_tbl[] = {
+ QMP_PHY_INIT_CFG(QSERDES_V3_TX_LANE_MODE_1, 0x06),
+ QMP_PHY_INIT_CFG(QSERDES_V3_TX_RES_CODE_LANE_OFFSET_TX, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V3_TX_RES_CODE_LANE_OFFSET_RX, 0x07),
+};
+
+static const struct qmp_phy_init_tbl sdm845_ufsphy_rx_tbl[] = {
+ QMP_PHY_INIT_CFG(QSERDES_V3_RX_SIGDET_LVL, 0x24),
+ QMP_PHY_INIT_CFG(QSERDES_V3_RX_SIGDET_CNTRL, 0x0f),
+ QMP_PHY_INIT_CFG(QSERDES_V3_RX_SIGDET_DEGLITCH_CNTRL, 0x1e),
+ QMP_PHY_INIT_CFG(QSERDES_V3_RX_RX_INTERFACE_MODE, 0x40),
+ QMP_PHY_INIT_CFG(QSERDES_V3_RX_UCDR_FASTLOCK_FO_GAIN, 0x0b),
+ QMP_PHY_INIT_CFG(QSERDES_V3_RX_RX_TERM_BW, 0x5b),
+ QMP_PHY_INIT_CFG(QSERDES_V3_RX_RX_EQU_ADAPTOR_CNTRL2, 0x06),
+ QMP_PHY_INIT_CFG(QSERDES_V3_RX_RX_EQU_ADAPTOR_CNTRL3, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V3_RX_RX_EQU_ADAPTOR_CNTRL4, 0x1b),
+ QMP_PHY_INIT_CFG(QSERDES_V3_RX_UCDR_SVS_SO_GAIN_HALF, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V3_RX_UCDR_SVS_SO_GAIN_QUARTER, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V3_RX_UCDR_SVS_SO_GAIN, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V3_RX_UCDR_SO_SATURATION_AND_ENABLE, 0x4b),
+ QMP_PHY_INIT_CFG(QSERDES_V3_RX_UCDR_PI_CONTROLS, 0x81),
+ QMP_PHY_INIT_CFG(QSERDES_V3_RX_UCDR_FASTLOCK_COUNT_LOW, 0x80),
+ QMP_PHY_INIT_CFG(QSERDES_V3_RX_RX_MODE_00, 0x59),
+};
+
+static const struct qmp_phy_init_tbl sdm845_ufsphy_pcs_tbl[] = {
+ QMP_PHY_INIT_CFG(QPHY_V3_PCS_RX_SIGDET_CTRL2, 0x6e),
+ QMP_PHY_INIT_CFG(QPHY_V3_PCS_TX_LARGE_AMP_DRV_LVL, 0x0a),
+ QMP_PHY_INIT_CFG(QPHY_V3_PCS_TX_SMALL_AMP_DRV_LVL, 0x02),
+ QMP_PHY_INIT_CFG(QPHY_V3_PCS_RX_SYM_RESYNC_CTRL, 0x03),
+ QMP_PHY_INIT_CFG(QPHY_V3_PCS_TX_MID_TERM_CTRL1, 0x43),
+ QMP_PHY_INIT_CFG(QPHY_V3_PCS_RX_SIGDET_CTRL1, 0x0f),
+ QMP_PHY_INIT_CFG(QPHY_V3_PCS_RX_MIN_HIBERN8_TIME, 0x9a),
+ QMP_PHY_INIT_CFG(QPHY_V3_PCS_MULTI_LANE_CTRL1, 0x02),
+};

/* struct qmp_phy_cfg - per-PHY initialization config */
struct qmp_phy_cfg {
@@ -654,6 +736,9 @@ struct qmp_phy_cfg {
/* Register offset of secondary tx/rx lanes for USB DP combo PHY */
unsigned int tx_b_lane_offset;
unsigned int rx_b_lane_offset;
+
+ /* true, if PCS block has no separate SW_RESET register */
+ bool no_pcs_sw_reset;
};

/**
@@ -750,6 +835,10 @@ static inline void qphy_clrbits(void __iomem *base, u32 offset, u32 val)
"aux", "cfg_ahb", "ref", "com_aux",
};

+static const char * const sdm845_ufs_phy_clk_l[] = {
+ "ref", "ref_aux",
+};
+
/* list of resets */
static const char * const msm8996_pciephy_reset_l[] = {
"phy", "common", "cfg",
@@ -919,6 +1008,35 @@ static inline void qphy_clrbits(void __iomem *base, u32 offset, u32 val)
.pwrdn_delay_max = POWER_DOWN_DELAY_US_MAX,
};

+static const struct qmp_phy_cfg sdm845_ufsphy_cfg = {
+ .type = PHY_TYPE_UFS,
+ .nlanes = 2,
+
+ .serdes_tbl = sdm845_ufsphy_serdes_tbl,
+ .serdes_tbl_num = ARRAY_SIZE(sdm845_ufsphy_serdes_tbl),
+ .tx_tbl = sdm845_ufsphy_tx_tbl,
+ .tx_tbl_num = ARRAY_SIZE(sdm845_ufsphy_tx_tbl),
+ .rx_tbl = sdm845_ufsphy_rx_tbl,
+ .rx_tbl_num = ARRAY_SIZE(sdm845_ufsphy_rx_tbl),
+ .pcs_tbl = sdm845_ufsphy_pcs_tbl,
+ .pcs_tbl_num = ARRAY_SIZE(sdm845_ufsphy_pcs_tbl),
+ .clk_list = sdm845_ufs_phy_clk_l,
+ .num_clks = ARRAY_SIZE(sdm845_ufs_phy_clk_l),
+ .vreg_list = qmp_phy_vreg_l,
+ .num_vregs = ARRAY_SIZE(qmp_phy_vreg_l),
+ .regs = sdm845_ufsphy_regs_layout,
+
+ .start_ctrl = SERDES_START,
+ .pwrdn_ctrl = SW_PWRDN,
+ .mask_pcs_ready = PCS_READY,
+
+ .is_dual_lane_phy = true,
+ .tx_b_lane_offset = 0x400,
+ .rx_b_lane_offset = 0x400,
+
+ .no_pcs_sw_reset = true,
+};
+
static void qcom_qmp_phy_configure(void __iomem *base,
const unsigned int *regs,
const struct qmp_phy_init_tbl tbl[],
@@ -1131,6 +1249,14 @@ static int qcom_qmp_phy_init(struct phy *phy)
qcom_qmp_phy_configure(pcs, cfg->regs, cfg->pcs_tbl, cfg->pcs_tbl_num);

/*
+ * UFS PHY requires the deassert of software reset before serdes start.
+ * For UFS PHYs that do not have software reset control bits, defer
+ * starting serdes until the power on callback.
+ */
+ if ((cfg->type == PHY_TYPE_UFS) && cfg->no_pcs_sw_reset)
+ goto out;
+
+ /*
* Pull out PHY from POWER DOWN state.
* This is active low enable signal to power-down PHY.
*/
@@ -1159,6 +1285,7 @@ static int qcom_qmp_phy_init(struct phy *phy)
}
qmp->phy_initialized = true;

+out:
return ret;

err_pcs_ready:
@@ -1181,7 +1308,8 @@ static int qcom_qmp_phy_exit(struct phy *phy)
clk_disable_unprepare(qphy->pipe_clk);

/* PHY reset */
- qphy_setbits(qphy->pcs, cfg->regs[QPHY_SW_RESET], SW_RESET);
+ if (!cfg->no_pcs_sw_reset)
+ qphy_setbits(qphy->pcs, cfg->regs[QPHY_SW_RESET], SW_RESET);

/* stop SerDes and Phy-Coding-Sublayer */
qphy_clrbits(qphy->pcs, cfg->regs[QPHY_START_CTRL], cfg->start_ctrl);
@@ -1199,6 +1327,44 @@ static int qcom_qmp_phy_exit(struct phy *phy)
return 0;
}

+static int qcom_qmp_phy_poweron(struct phy *phy)
+{
+ struct qmp_phy *qphy = phy_get_drvdata(phy);
+ struct qcom_qmp *qmp = qphy->qmp;
+ const struct qmp_phy_cfg *cfg = qmp->cfg;
+ void __iomem *pcs = qphy->pcs;
+ void __iomem *status;
+ unsigned int mask, val;
+ int ret = 0;
+
+ if (cfg->type != PHY_TYPE_UFS)
+ return 0;
+
+ /*
+ * For UFS PHY that has not software reset control, serdes start
+ * should only happen when UFS driver explicitly calls phy_power_on
+ * after it deasserts software reset.
+ */
+ if (cfg->no_pcs_sw_reset && !qmp->phy_initialized &&
+ (qmp->init_count != 0)) {
+ /* start SerDes and Phy-Coding-Sublayer */
+ qphy_setbits(pcs, cfg->regs[QPHY_START_CTRL], cfg->start_ctrl);
+
+ status = pcs + cfg->regs[QPHY_PCS_READY_STATUS];
+ mask = cfg->mask_pcs_ready;
+
+ ret = readl_poll_timeout(status, val, !(val & mask), 1,
+ PHY_INIT_COMPLETE_TIMEOUT);
+ if (ret) {
+ dev_err(qmp->dev, "phy initialization timed-out\n");
+ return ret;
+ }
+ qmp->phy_initialized = true;
+ }
+
+ return ret;
+}
+
static int qcom_qmp_phy_set_mode(struct phy *phy, enum phy_mode mode)
{
struct qmp_phy *qphy = phy_get_drvdata(phy);
@@ -1428,6 +1594,7 @@ static int phy_pipe_clk_register(struct qcom_qmp *qmp, struct device_node *np)
static const struct phy_ops qcom_qmp_phy_gen_ops = {
.init = qcom_qmp_phy_init,
.exit = qcom_qmp_phy_exit,
+ .power_on = qcom_qmp_phy_poweron,
.set_mode = qcom_qmp_phy_set_mode,
.owner = THIS_MODULE,
};
@@ -1533,6 +1700,9 @@ int qcom_qmp_phy_create(struct device *dev, struct device_node *np, int id)
}, {
.compatible = "qcom,sdm845-qmp-usb3-uni-phy",
.data = &qmp_v3_usb3_uniphy_cfg,
+ }, {
+ .compatible = "qcom,sdm845-qmp-ufs-phy",
+ .data = &sdm845_ufsphy_cfg,
},
{ },
};
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.h b/drivers/phy/qualcomm/phy-qcom-qmp.h
index 5d78d43..d201cc3 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.h
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.h
@@ -184,6 +184,8 @@
#define QSERDES_V3_COM_VCO_TUNE2_MODE0 0x0f8
#define QSERDES_V3_COM_VCO_TUNE1_MODE1 0x0fc
#define QSERDES_V3_COM_VCO_TUNE2_MODE1 0x100
+#define QSERDES_V3_COM_VCO_TUNE_INITVAL1 0x104
+#define QSERDES_V3_COM_VCO_TUNE_INITVAL2 0x108
#define QSERDES_V3_COM_VCO_TUNE_TIMER1 0x11c
#define QSERDES_V3_COM_VCO_TUNE_TIMER2 0x120
#define QSERDES_V3_COM_CLK_SELECT 0x138
@@ -211,8 +213,13 @@
/* Only for QMP V3 PHY - RX registers */
#define QSERDES_V3_RX_UCDR_SO_GAIN_HALF 0x00c
#define QSERDES_V3_RX_UCDR_SO_GAIN 0x014
+#define QSERDES_V3_RX_UCDR_SVS_SO_GAIN_HALF 0x024
+#define QSERDES_V3_RX_UCDR_SVS_SO_GAIN_QUARTER 0x028
+#define QSERDES_V3_RX_UCDR_SVS_SO_GAIN 0x02c
#define QSERDES_V3_RX_UCDR_FASTLOCK_FO_GAIN 0x030
#define QSERDES_V3_RX_UCDR_SO_SATURATION_AND_ENABLE 0x034
+#define QSERDES_V3_RX_UCDR_FASTLOCK_COUNT_LOW 0x03c
+#define QSERDES_V3_RX_UCDR_PI_CONTROLS 0x044
#define QSERDES_V3_RX_RX_TERM_BW 0x07c
#define QSERDES_V3_RX_VGA_CAL_CNTRL1 0x0bc
#define QSERDES_V3_RX_VGA_CAL_CNTRL2 0x0c0
@@ -239,6 +246,8 @@
#define QPHY_V3_PCS_TXMGN_V3 0x018
#define QPHY_V3_PCS_TXMGN_V4 0x01c
#define QPHY_V3_PCS_TXMGN_LS 0x020
+#define QPHY_V3_PCS_TX_LARGE_AMP_DRV_LVL 0x02c
+#define QPHY_V3_PCS_TX_SMALL_AMP_DRV_LVL 0x034
#define QPHY_V3_PCS_TXDEEMPH_M6DB_V0 0x024
#define QPHY_V3_PCS_TXDEEMPH_M3P5DB_V0 0x028
#define QPHY_V3_PCS_TXDEEMPH_M6DB_V1 0x02c
@@ -275,6 +284,12 @@
#define QPHY_V3_PCS_FLL_CNT_VAL_L 0x0cc
#define QPHY_V3_PCS_FLL_CNT_VAL_H_TOL 0x0d0
#define QPHY_V3_PCS_FLL_MAN_CODE 0x0d4
+#define QPHY_V3_PCS_RX_SYM_RESYNC_CTRL 0x134
+#define QPHY_V3_PCS_RX_MIN_HIBERN8_TIME 0x138
+#define QPHY_V3_PCS_RX_SIGDET_CTRL1 0x13c
+#define QPHY_V3_PCS_RX_SIGDET_CTRL2 0x140
+#define QPHY_V3_PCS_TX_MID_TERM_CTRL1 0x1bc
+#define QPHY_V3_PCS_MULTI_LANE_CTRL1 0x1c4
#define QPHY_V3_PCS_RX_SIGDET_LVL 0x1d8
#define QPHY_V3_PCS_REFGEN_REQ_CONFIG1 0x20c
#define QPHY_V3_PCS_REFGEN_REQ_CONFIG2 0x210
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


2018-08-01 02:22:11

by Can Guo

[permalink] [raw]
Subject: Re: [PATCH v8 0/5] Support for Qualcomm UFS QMP PHY on SDM845

On 2018-07-31 18:09, Can Guo wrote:
> This patch series adds support for UFS QMP PHY on SDM845 and the
> compatible string for it. This patch series depends on the current
> proposed QMP V3 USB3 UNI PHY support for sdm845 driver [1], on
> the DT bindings for the QMP V3 USB3 PHYs based dirver [2], and also
> rebased on updated pipe_clk initialization sequence [3]. This series
> can only be merged once the dependent patches do.
> [1]
> http://lists-archives.com/linux-kernel/29071659-dt-bindings-phy-qcom-qmp-update-bindings-for-sdm845.html
> [2]
> http://lists-archives.com/linux-kernel/29071660-phy-qcom-qmp-add-qmp-v3-usb3-uni-phy-support-for-sdm845.html
> [3] https://patchwork.kernel.org/patch/10376551/
>
> Changes since v7:
> - Add one new change to update UFS PHY power on sequence
> - Incorporated review comments from Evan, Vivek and Manu.
>
> Changes since v6:
> - Add one new change to clean up some structs and field
> - Updates the PHY power control sequence.
> - Incorporated review comments from Vivek and Manu.
>
> Changes since v5:
> - Updates the PHY power control sequence.
> - Updates UFS PHY power on condition check.
>
> Changes since v4:
> - Adds 'ref_aux' clock back to SDM845 UFS PHY clock list.
> - Power on PHY before serdes configuration starts.
> - Updates the UFS PHY initialization sequence.
> - Updates a few UFS PHY registers.
> - Incorporated review comments from Vivek and Manu.
>
> Changes since v3:
> - Incorporated review comments from Vivek and Rob.
>
> Changes since v2:
> - Incorporated review comments from Vivek and Rob.
> - Remove "ref_aux" from sdm845 ufs phy clock list structure.
>
> Changes since v1:
> - Incorporated review comments from Vivek and Manu.
> - Update the commit title of patch 2.
>
>
> Can Guo (5):
> phy: Update PHY power control sequence
> phy: General struct and field cleanup
> phy: Add QMP phy based UFS phy support for sdm845
> scsi: ufs: Power on phy after it is initialized
> dt-bindings: phy-qcom-qmp: Add UFS phy compatible string for sdm845
>
> .../devicetree/bindings/phy/qcom-qmp-phy.txt | 4 +-
> drivers/phy/qualcomm/phy-qcom-qmp.c | 216
> +++++++++++++++++++--
> drivers/phy/qualcomm/phy-qcom-qmp.h | 15 ++
> drivers/scsi/ufs/ufs-qcom.c | 4 +-
> drivers/scsi/ufs/ufs-qcom.h | 1 +
> 5 files changed, 219 insertions(+), 21 deletions(-)

2018-08-08 21:57:21

by Evan Green

[permalink] [raw]
Subject: Re: [PATCH v8 3/5] phy: Add QMP phy based UFS phy support for sdm845

On Tue, Jul 31, 2018 at 3:09 AM Can Guo <[email protected]> wrote:
>
> Add UFS PHY support to make SDM845 UFS work with common PHY framework.
>
> Signed-off-by: Can Guo <[email protected]>
> ---
> drivers/phy/qualcomm/phy-qcom-qmp.c | 172 +++++++++++++++++++++++++++++++++++-
> drivers/phy/qualcomm/phy-qcom-qmp.h | 15 ++++
> 2 files changed, 186 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c
> index 9be9754..de7ff18 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
...
> static void qcom_qmp_phy_configure(void __iomem *base,
> const unsigned int *regs,
> const struct qmp_phy_init_tbl tbl[],
> @@ -1131,6 +1249,14 @@ static int qcom_qmp_phy_init(struct phy *phy)
> qcom_qmp_phy_configure(pcs, cfg->regs, cfg->pcs_tbl, cfg->pcs_tbl_num);
>
> /*
> + * UFS PHY requires the deassert of software reset before serdes start.
> + * For UFS PHYs that do not have software reset control bits, defer
> + * starting serdes until the power on callback.
> + */

I'm relatively thick when it comes to PHYs, but I'm a little confused.
The sequence of code right below this (not shown) looks like it is
deasserting reset before starting serdes, seemingly doing what the
comment wants. I guess the problem is the lack of SW reset? So then
you defer serdes start until UFS does... something. Can you explain
how deferring to after UFS HC init actually helps? Is it the UFS HC
that releases reset on the PHY?

I was hoping the next patch would help, but I'm still confused. It
looks like you've added a call to phy_power_on in
ufs_qcom_setup_clocks, but there's also one still in
ufs_qcom_power_up_sequence. What does the original phy_power_on in
ufs_qcom_power_up_sequence do now? It seems like that one would do the
power on too early, and then your new added call in
ufs_qcom_setup_clocks would do nothing.


> + if ((cfg->type == PHY_TYPE_UFS) && cfg->no_pcs_sw_reset)
> + goto out;
> +
> + /*
> * Pull out PHY from POWER DOWN state.
> * This is active low enable signal to power-down PHY.
> */
> @@ -1159,6 +1285,7 @@ static int qcom_qmp_phy_init(struct phy *phy)
> }
> qmp->phy_initialized = true;
>
> +out:
> return ret;
>
> err_pcs_ready:
> @@ -1181,7 +1308,8 @@ static int qcom_qmp_phy_exit(struct phy *phy)
> clk_disable_unprepare(qphy->pipe_clk);
>
> /* PHY reset */
> - qphy_setbits(qphy->pcs, cfg->regs[QPHY_SW_RESET], SW_RESET);
> + if (!cfg->no_pcs_sw_reset)
> + qphy_setbits(qphy->pcs, cfg->regs[QPHY_SW_RESET], SW_RESET);
>
> /* stop SerDes and Phy-Coding-Sublayer */
> qphy_clrbits(qphy->pcs, cfg->regs[QPHY_START_CTRL], cfg->start_ctrl);
> @@ -1199,6 +1327,44 @@ static int qcom_qmp_phy_exit(struct phy *phy)
> return 0;
> }
>
> +static int qcom_qmp_phy_poweron(struct phy *phy)
> +{
> + struct qmp_phy *qphy = phy_get_drvdata(phy);
> + struct qcom_qmp *qmp = qphy->qmp;
> + const struct qmp_phy_cfg *cfg = qmp->cfg;
> + void __iomem *pcs = qphy->pcs;
> + void __iomem *status;
> + unsigned int mask, val;
> + int ret = 0;
> +
> + if (cfg->type != PHY_TYPE_UFS)
> + return 0;
> +
> + /*
> + * For UFS PHY that has not software reset control, serdes start
> + * should only happen when UFS driver explicitly calls phy_power_on
> + * after it deasserts software reset.
> + */
> + if (cfg->no_pcs_sw_reset && !qmp->phy_initialized &&
> + (qmp->init_count != 0)) {
> + /* start SerDes and Phy-Coding-Sublayer */
> + qphy_setbits(pcs, cfg->regs[QPHY_START_CTRL], cfg->start_ctrl);
> +
> + status = pcs + cfg->regs[QPHY_PCS_READY_STATUS];
> + mask = cfg->mask_pcs_ready;
> +
> + ret = readl_poll_timeout(status, val, !(val & mask), 1,
> + PHY_INIT_COMPLETE_TIMEOUT);
> + if (ret) {
> + dev_err(qmp->dev, "phy initialization timed-out\n");
> + return ret;
> + }
> + qmp->phy_initialized = true;
> + }
> +
> + return ret;
> +}
> +
> static int qcom_qmp_phy_set_mode(struct phy *phy, enum phy_mode mode)
> {
> struct qmp_phy *qphy = phy_get_drvdata(phy);
> @@ -1428,6 +1594,7 @@ static int phy_pipe_clk_register(struct qcom_qmp *qmp, struct device_node *np)
> static const struct phy_ops qcom_qmp_phy_gen_ops = {
> .init = qcom_qmp_phy_init,
> .exit = qcom_qmp_phy_exit,
> + .power_on = qcom_qmp_phy_poweron,

The power_on function without corresponding power_off still makes me
nervous. But with phy_initialized and init_count members, I guess this
works.

> .set_mode = qcom_qmp_phy_set_mode,
> .owner = THIS_MODULE,
> };
> @@ -1533,6 +1700,9 @@ int qcom_qmp_phy_create(struct device *dev, struct device_node *np, int id)
> }, {
> .compatible = "qcom,sdm845-qmp-usb3-uni-phy",
> .data = &qmp_v3_usb3_uniphy_cfg,
> + }, {
> + .compatible = "qcom,sdm845-qmp-ufs-phy",
> + .data = &sdm845_ufsphy_cfg,
> },
> { },
> };

2018-08-09 17:27:49

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH v8 3/5] phy: Add QMP phy based UFS phy support for sdm845

Hi Evan,


On 8/9/2018 3:25 AM, Evan Green wrote:
> On Tue, Jul 31, 2018 at 3:09 AM Can Guo <[email protected]> wrote:
>> Add UFS PHY support to make SDM845 UFS work with common PHY framework.
>>
>> Signed-off-by: Can Guo <[email protected]>
>> ---
>> drivers/phy/qualcomm/phy-qcom-qmp.c | 172 +++++++++++++++++++++++++++++++++++-
>> drivers/phy/qualcomm/phy-qcom-qmp.h | 15 ++++
>> 2 files changed, 186 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c
>> index 9be9754..de7ff18 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
> ...
>> static void qcom_qmp_phy_configure(void __iomem *base,
>> const unsigned int *regs,
>> const struct qmp_phy_init_tbl tbl[],
>> @@ -1131,6 +1249,14 @@ static int qcom_qmp_phy_init(struct phy *phy)
>> qcom_qmp_phy_configure(pcs, cfg->regs, cfg->pcs_tbl, cfg->pcs_tbl_num);
>>
>> /*
>> + * UFS PHY requires the deassert of software reset before serdes start.
>> + * For UFS PHYs that do not have software reset control bits, defer
>> + * starting serdes until the power on callback.
>> + */
> I'm relatively thick when it comes to PHYs, but I'm a little confused.
> The sequence of code right below this (not shown) looks like it is
> deasserting reset before starting serdes, seemingly doing what the
> comment wants. I guess the problem is the lack of SW reset? So then
> you defer serdes start until UFS does... something. Can you explain
> how deferring to after UFS HC init actually helps? Is it the UFS HC
> that releases reset on the PHY?

As you can see in [1], the ufs first asserts the sw_reset, then phy
initialization is done.
This phy_init() is just programming the phy registers. Now as per the H/W
programming doc, we can't start the phy until we de-assert the sw_reset.
So the sequence as per the programming doc should be:

assert SW_reset --> program phy serdes/tx/rx/pcs registers --> deassert
SW_reset --> start serdes --> test PCS status

That's the reason that serdes_start has been moved to phy_power_on(), as
that seemed
a more cleaner way of handling the above sequence.
UFS HC init doesn't help more than this in terms of phy initialization.

>
> I was hoping the next patch would help, but I'm still confused. It
> looks like you've added a call to phy_power_on in
> ufs_qcom_setup_clocks, but there's also one still in
> ufs_qcom_power_up_sequence. What does the original phy_power_on in
> ufs_qcom_power_up_sequence do now? It seems like that one would do the
> power on too early, and then your new added call in
> ufs_qcom_setup_clocks would do nothing.

I think [patch 4/5] of this series handles this. We skip the
phy_power_on until
we do phy_init.
phy_power_on/off() in setup_clocks() is also used for suspend/resume case
and that's the reason you see couple of phy_power_on(). Patch 4/5 should
handle
this now.

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/scsi/ufs/ufs-qcom.c#n268

[snip]

Regards
Vivek

2018-08-09 18:01:33

by Evan Green

[permalink] [raw]
Subject: Re: [PATCH v8 3/5] phy: Add QMP phy based UFS phy support for sdm845

On Thu, Aug 9, 2018 at 10:26 AM Vivek Gautam
<[email protected]> wrote:
>
> Hi Evan,
>
>
> On 8/9/2018 3:25 AM, Evan Green wrote:
> > On Tue, Jul 31, 2018 at 3:09 AM Can Guo <[email protected]> wrote:
> >> Add UFS PHY support to make SDM845 UFS work with common PHY framework.
> >>
> >> Signed-off-by: Can Guo <[email protected]>
> >> ---
> >> drivers/phy/qualcomm/phy-qcom-qmp.c | 172 +++++++++++++++++++++++++++++++++++-
> >> drivers/phy/qualcomm/phy-qcom-qmp.h | 15 ++++
> >> 2 files changed, 186 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c
> >> index 9be9754..de7ff18 100644
> >> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c
> >> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
> > ...
> >> static void qcom_qmp_phy_configure(void __iomem *base,
> >> const unsigned int *regs,
> >> const struct qmp_phy_init_tbl tbl[],
> >> @@ -1131,6 +1249,14 @@ static int qcom_qmp_phy_init(struct phy *phy)
> >> qcom_qmp_phy_configure(pcs, cfg->regs, cfg->pcs_tbl, cfg->pcs_tbl_num);
> >>
> >> /*
> >> + * UFS PHY requires the deassert of software reset before serdes start.
> >> + * For UFS PHYs that do not have software reset control bits, defer
> >> + * starting serdes until the power on callback.
> >> + */
> > I'm relatively thick when it comes to PHYs, but I'm a little confused.
> > The sequence of code right below this (not shown) looks like it is
> > deasserting reset before starting serdes, seemingly doing what the
> > comment wants. I guess the problem is the lack of SW reset? So then
> > you defer serdes start until UFS does... something. Can you explain
> > how deferring to after UFS HC init actually helps? Is it the UFS HC
> > that releases reset on the PHY?
>
> As you can see in [1], the ufs first asserts the sw_reset, then phy
> initialization is done.
> This phy_init() is just programming the phy registers. Now as per the H/W
> programming doc, we can't start the phy until we de-assert the sw_reset.
> So the sequence as per the programming doc should be:
>
> assert SW_reset --> program phy serdes/tx/rx/pcs registers --> deassert
> SW_reset --> start serdes --> test PCS status
>
> That's the reason that serdes_start has been moved to phy_power_on(), as
> that seemed
> a more cleaner way of handling the above sequence.
> UFS HC init doesn't help more than this in terms of phy initialization.

Ok that makes sense. Thank you for the explanation.

>
> >
> > I was hoping the next patch would help, but I'm still confused. It
> > looks like you've added a call to phy_power_on in
> > ufs_qcom_setup_clocks, but there's also one still in
> > ufs_qcom_power_up_sequence. What does the original phy_power_on in
> > ufs_qcom_power_up_sequence do now? It seems like that one would do the
> > power on too early, and then your new added call in
> > ufs_qcom_setup_clocks would do nothing.
>
> I think [patch 4/5] of this series handles this. We skip the
> phy_power_on until
> we do phy_init.
> phy_power_on/off() in setup_clocks() is also used for suspend/resume case
> and that's the reason you see couple of phy_power_on(). Patch 4/5 should
> handle
> this now.

Ok got it. I was confused about the ordering. setup_clocks is actually
called first (via __ufshcd_setup_clocks > ufshcd_hba_init >
ufshcd_init), which is why you need the boolean to defer this power on
until later.

Reviewed-by: Evan Green <[email protected]>

2018-08-09 18:02:22

by Evan Green

[permalink] [raw]
Subject: Re: [PATCH v8 4/5] scsi: ufs: Power on phy after it is initialized

On Tue, Jul 31, 2018 at 3:09 AM Can Guo <[email protected]> wrote:
>
> Before UFS PHY is initialized, powering on it has no effect but increases
> the PHY's power on count. Then when power on PHY to enable it after phy
> initialization where it is really needed, as PHY's power on count is not
> zero, again it would do nothing but directly return and leaves the PHY
> disabled. This change adds condition checks to phy power on calls which
> happen before phy initialization.
>
> Signed-off-by: Can Guo <[email protected]>
> ---
> drivers/scsi/ufs/ufs-qcom.c | 4 +++-
> drivers/scsi/ufs/ufs-qcom.h | 1 +
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
> index 2b38db2..4e9d526 100644
> --- a/drivers/scsi/ufs/ufs-qcom.c
> +++ b/drivers/scsi/ufs/ufs-qcom.c
> @@ -288,6 +288,7 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba)
> __func__, ret);
> goto out;
> }
> + host->is_phy_init = true;
>
> /* De-assert PHY reset and start serdes */
> ufs_qcom_deassert_reset(hba);
> @@ -1147,7 +1148,8 @@ static int ufs_qcom_setup_clocks(struct ufs_hba *hba, bool on,
> return 0;
>
> if (on && (status == POST_CHANGE)) {
> - phy_power_on(host->generic_phy);
> + if (host->is_phy_init)
> + phy_power_on(host->generic_phy);
>
> /* enable the device ref clock for HS mode*/
> if (ufshcd_is_hs_mode(&hba->pwr_info))
> diff --git a/drivers/scsi/ufs/ufs-qcom.h b/drivers/scsi/ufs/ufs-qcom.h
> index 295f4be..5747aa3 100644
> --- a/drivers/scsi/ufs/ufs-qcom.h
> +++ b/drivers/scsi/ufs/ufs-qcom.h
> @@ -242,6 +242,7 @@ struct ufs_qcom_host {
> /* Bitmask for enabling debug prints */
> u32 dbg_print_en;
> struct ufs_qcom_testbus testbus;
> + bool is_phy_init;
> };
>
> static inline u32
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

Reviewed-by: Evan Green <[email protected]>

2018-08-30 06:18:20

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v8 0/5] Support for Qualcomm UFS QMP PHY on SDM845

On Tue 31 Jul 03:09 PDT 2018, Can Guo wrote:

> This patch series adds support for UFS QMP PHY on SDM845 and the
> compatible string for it. This patch series depends on the current
> proposed QMP V3 USB3 UNI PHY support for sdm845 driver [1], on
> the DT bindings for the QMP V3 USB3 PHYs based dirver [2], and also
> rebased on updated pipe_clk initialization sequence [3]. This series
> can only be merged once the dependent patches do.
> [1] http://lists-archives.com/linux-kernel/29071659-dt-bindings-phy-qcom-qmp-update-bindings-for-sdm845.html
> [2] http://lists-archives.com/linux-kernel/29071660-phy-qcom-qmp-add-qmp-v3-usb3-uni-phy-support-for-sdm845.html
> [3] https://patchwork.kernel.org/patch/10376551/
>

Hi Can,

We still have the issue that the older non-QMP phy driver takes a direct
call to ufs_qcom_phy_set_tx_lane_enable(), which means that pointing
UFSHCD to this phy implementation results in a kernel panic.

I do however think this series is good in itself and would like to see
it merged. As any fix for this problem would be done on top of the
patches in this series.

Tested-by: Bjorn Andersson <[email protected]>

Regards,
Bjorn

2018-08-30 10:46:47

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH v8 0/5] Support for Qualcomm UFS QMP PHY on SDM845

On Thu, Aug 30, 2018 at 11:46 AM Bjorn Andersson
<[email protected]> wrote:
>
> On Tue 31 Jul 03:09 PDT 2018, Can Guo wrote:
>
> > This patch series adds support for UFS QMP PHY on SDM845 and the
> > compatible string for it. This patch series depends on the current
> > proposed QMP V3 USB3 UNI PHY support for sdm845 driver [1], on
> > the DT bindings for the QMP V3 USB3 PHYs based dirver [2], and also
> > rebased on updated pipe_clk initialization sequence [3]. This series
> > can only be merged once the dependent patches do.
> > [1] http://lists-archives.com/linux-kernel/29071659-dt-bindings-phy-qcom-qmp-update-bindings-for-sdm845.html
> > [2] http://lists-archives.com/linux-kernel/29071660-phy-qcom-qmp-add-qmp-v3-usb3-uni-phy-support-for-sdm845.html
> > [3] https://patchwork.kernel.org/patch/10376551/
> >
>
> Hi Can,
>
> We still have the issue that the older non-QMP phy driver takes a direct
> call to ufs_qcom_phy_set_tx_lane_enable(), which means that pointing
> UFSHCD to this phy implementation results in a kernel panic.

Hi Bjorn,

I have a series that removes couple of these function calls -
ufs_qcom_phy_set_tx_lane_enable(), and
ufs_qcom_phy_save_controller_version() from ufs-qcom, and
make phy-qcom-ufs-qmp-20nm as broken.
I can post it out if that sounds okay.

Best regards
Vivek

[snip]


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

2018-08-31 01:11:59

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH v8 0/5] Support for Qualcomm UFS QMP PHY on SDM845

On Thu 30 Aug 03:44 PDT 2018, Vivek Gautam wrote:

> On Thu, Aug 30, 2018 at 11:46 AM Bjorn Andersson
> <[email protected]> wrote:
> >
> > On Tue 31 Jul 03:09 PDT 2018, Can Guo wrote:
> >
> > > This patch series adds support for UFS QMP PHY on SDM845 and the
> > > compatible string for it. This patch series depends on the current
> > > proposed QMP V3 USB3 UNI PHY support for sdm845 driver [1], on
> > > the DT bindings for the QMP V3 USB3 PHYs based dirver [2], and also
> > > rebased on updated pipe_clk initialization sequence [3]. This series
> > > can only be merged once the dependent patches do.
> > > [1] http://lists-archives.com/linux-kernel/29071659-dt-bindings-phy-qcom-qmp-update-bindings-for-sdm845.html
> > > [2] http://lists-archives.com/linux-kernel/29071660-phy-qcom-qmp-add-qmp-v3-usb3-uni-phy-support-for-sdm845.html
> > > [3] https://patchwork.kernel.org/patch/10376551/
> > >
> >
> > Hi Can,
> >
> > We still have the issue that the older non-QMP phy driver takes a direct
> > call to ufs_qcom_phy_set_tx_lane_enable(), which means that pointing
> > UFSHCD to this phy implementation results in a kernel panic.
>
> Hi Bjorn,
>
> I have a series that removes couple of these function calls -
> ufs_qcom_phy_set_tx_lane_enable(), and
> ufs_qcom_phy_save_controller_version() from ufs-qcom, and
> make phy-qcom-ufs-qmp-20nm as broken.
> I can post it out if that sounds okay.
>

That sounds really good, please do sent that out.

Regards,
Bjorn