diff mbox series

[5/9] phy: qcom-qmp: use correct values for ipq8074 gen2 pcie phy init

Message ID 1593940680-2363-6-git-send-email-sivaprak@codeaurora.org (mailing list archive)
State Superseded, archived
Headers show
Series Add PCIe support for IPQ8074 | expand

Commit Message

Sivaprakash Murugesan July 5, 2020, 9:17 a.m. UTC
There were some problem in ipq8074 gen2 pcie phy init sequence, fix
these to make gen2 pcie port on ipq8074 to work.

Fixes: eef243d04b2b6 ("phy: qcom-qmp: Add support for IPQ8074")

Cc: stable@vger.kernel.org
Co-developed-by: Selvam Sathappan Periakaruppan <speriaka@codeaurora.org>
Signed-off-by: Selvam Sathappan Periakaruppan <speriaka@codeaurora.org>
Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
 drivers/phy/qualcomm/phy-qcom-qmp.c | 16 +++++++++-------
 drivers/phy/qualcomm/phy-qcom-qmp.h |  2 ++
 2 files changed, 11 insertions(+), 7 deletions(-)

Comments

Vinod Koul July 13, 2020, 5:55 a.m. UTC | #1
On 05-07-20, 14:47, Sivaprakash Murugesan wrote:
> There were some problem in ipq8074 gen2 pcie phy init sequence, fix

Can you please describe these problems, it would help review to
understand the issues and also for future reference to you

> these to make gen2 pcie port on ipq8074 to work.
> 
> Fixes: eef243d04b2b6 ("phy: qcom-qmp: Add support for IPQ8074")
> 
> Cc: stable@vger.kernel.org
> Co-developed-by: Selvam Sathappan Periakaruppan <speriaka@codeaurora.org>
> Signed-off-by: Selvam Sathappan Periakaruppan <speriaka@codeaurora.org>
> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp.c | 16 +++++++++-------
>  drivers/phy/qualcomm/phy-qcom-qmp.h |  2 ++
>  2 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c
> index e91040af3394..ba277136f52b 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
> @@ -504,8 +504,8 @@ static const struct qmp_phy_init_tbl ipq8074_pcie_serdes_tbl[] = {
>  	QMP_PHY_INIT_CFG(QSERDES_COM_BG_TRIM, 0xf),
>  	QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP_EN, 0x1),
>  	QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_MAP, 0x0),
> -	QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_TIMER1, 0x1f),
> -	QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_TIMER2, 0x3f),
> +	QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_TIMER1, 0xff),
> +	QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_TIMER2, 0x1f),
>  	QMP_PHY_INIT_CFG(QSERDES_COM_CMN_CONFIG, 0x6),
>  	QMP_PHY_INIT_CFG(QSERDES_COM_PLL_IVCO, 0xf),
>  	QMP_PHY_INIT_CFG(QSERDES_COM_HSCLK_SEL, 0x0),
> @@ -531,7 +531,6 @@ static const struct qmp_phy_init_tbl ipq8074_pcie_serdes_tbl[] = {
>  	QMP_PHY_INIT_CFG(QSERDES_COM_INTEGLOOP_GAIN1_MODE0, 0x0),
>  	QMP_PHY_INIT_CFG(QSERDES_COM_INTEGLOOP_GAIN0_MODE0, 0x80),
>  	QMP_PHY_INIT_CFG(QSERDES_COM_BIAS_EN_CTRL_BY_PSM, 0x1),
> -	QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_CTRL, 0xa),
>  	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_EN_CENTER, 0x1),
>  	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_PER1, 0x31),
>  	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_PER2, 0x1),
> @@ -540,7 +539,6 @@ static const struct qmp_phy_init_tbl ipq8074_pcie_serdes_tbl[] = {
>  	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_STEP_SIZE1, 0x2f),
>  	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_STEP_SIZE2, 0x19),
>  	QMP_PHY_INIT_CFG(QSERDES_COM_CLK_EP_DIV, 0x19),
> -	QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_CNTRL, 0x7),
>  };
>  
>  static const struct qmp_phy_init_tbl ipq8074_pcie_tx_tbl[] = {
> @@ -548,6 +546,8 @@ static const struct qmp_phy_init_tbl ipq8074_pcie_tx_tbl[] = {
>  	QMP_PHY_INIT_CFG(QSERDES_TX_LANE_MODE, 0x6),
>  	QMP_PHY_INIT_CFG(QSERDES_TX_RES_CODE_LANE_OFFSET, 0x2),
>  	QMP_PHY_INIT_CFG(QSERDES_TX_RCV_DETECT_LVL_2, 0x12),
> +	QMP_PHY_INIT_CFG(QSERDES_TX_EMP_POST1_LVL, 0x36),
> +	QMP_PHY_INIT_CFG(QSERDES_TX_SLEW_CNTL, 0x0a),
>  };
>  
>  static const struct qmp_phy_init_tbl ipq8074_pcie_rx_tbl[] = {
> @@ -558,7 +558,6 @@ static const struct qmp_phy_init_tbl ipq8074_pcie_rx_tbl[] = {
>  	QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL4, 0xdb),
>  	QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_SO_SATURATION_AND_ENABLE, 0x4b),
>  	QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_SO_GAIN, 0x4),
> -	QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_SO_GAIN_HALF, 0x4),
>  };
>  
>  static const struct qmp_phy_init_tbl ipq8074_pcie_pcs_tbl[] = {
> @@ -1673,6 +1672,9 @@ static const struct qmp_phy_cfg msm8996_usb3phy_cfg = {
>  	.pwrdn_ctrl		= SW_PWRDN,
>  };
>  
> +static const char * const ipq8074_pciephy_clk_l[] = {
> +	"aux", "cfg_ahb",
> +};
>  /* list of resets */
>  static const char * const ipq8074_pciephy_reset_l[] = {
>  	"phy", "common",
> @@ -1690,8 +1692,8 @@ static const struct qmp_phy_cfg ipq8074_pciephy_cfg = {
>  	.rx_tbl_num		= ARRAY_SIZE(ipq8074_pcie_rx_tbl),
>  	.pcs_tbl		= ipq8074_pcie_pcs_tbl,
>  	.pcs_tbl_num		= ARRAY_SIZE(ipq8074_pcie_pcs_tbl),
> -	.clk_list		= NULL,
> -	.num_clks		= 0,
> +	.clk_list		= ipq8074_pciephy_clk_l,
> +	.num_clks		= ARRAY_SIZE(ipq8074_pciephy_clk_l),

I see patch is modifying some register values and then adding clks, in
the absence of proper patch description it is extremely hard to
understand what is going on..

>  	.reset_list		= ipq8074_pciephy_reset_l,
>  	.num_resets		= ARRAY_SIZE(ipq8074_pciephy_reset_l),
>  	.vreg_list		= NULL,
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.h b/drivers/phy/qualcomm/phy-qcom-qmp.h
> index 6d017a0c0c8d..832b3d098403 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp.h
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.h
> @@ -77,6 +77,8 @@
>  #define QSERDES_COM_CORECLK_DIV_MODE1			0x1bc
>  
>  /* Only for QMP V2 PHY - TX registers */
> +#define QSERDES_TX_EMP_POST1_LVL			0x018
> +#define QSERDES_TX_SLEW_CNTL				0x040
>  #define QSERDES_TX_RES_CODE_LANE_OFFSET			0x054
>  #define QSERDES_TX_DEBUG_BUS_SEL			0x064
>  #define QSERDES_TX_HIGHZ_TRANSCEIVEREN_BIAS_DRVR_EN	0x068
> -- 
> 2.7.4
Sivaprakash Murugesan July 29, 2020, 6:45 a.m. UTC | #2
On 7/13/2020 11:25 AM, Vinod Koul wrote:
> On 05-07-20, 14:47, Sivaprakash Murugesan wrote:
>> There were some problem in ipq8074 gen2 pcie phy init sequence, fix
> Can you please describe these problems, it would help review to
> understand the issues and also for future reference to you

Hi Vinod,

As you mentioned we are updating few register values

and also adding clocks and resets.

the register values are given by the Hardware team and there

is some fine tuning values are provided by Hardware team for the

issues we faced downstream.

Also, few register values are typos for example QSERDES_RX_SIGDET_CNTRL

is a rx register it was wrongly in serdes table.

I will try to mention these details in next patch.
Vinod Koul Aug. 3, 2020, 11:03 a.m. UTC | #3
Hi Sivaprakash,

On 29-07-20, 12:15, Sivaprakash Murugesan wrote:
> 
> On 7/13/2020 11:25 AM, Vinod Koul wrote:
> > On 05-07-20, 14:47, Sivaprakash Murugesan wrote:
> > > There were some problem in ipq8074 gen2 pcie phy init sequence, fix
> > Can you please describe these problems, it would help review to
> > understand the issues and also for future reference to you
> 
> Hi Vinod,
> 
> As you mentioned we are updating few register values
> 
> and also adding clocks and resets.
> 
> the register values are given by the Hardware team and there
> 
> is some fine tuning values are provided by Hardware team for the
> 
> issues we faced downstream.
> 
> Also, few register values are typos for example QSERDES_RX_SIGDET_CNTRL
> 
> is a rx register it was wrongly in serdes table.
> 
> I will try to mention these details in next patch.

The right thing to do would be a change per patch explaining the reason.
For example, fixing typos in QSERDES_RX_SIGDET_CNTRL, then another to
update tuning values based on hw recommendations. Clocks and reset
should be different patch

This helps us review each change for what it does and helps you down the
line to figure why a line of code was changed

HTH
diff mbox series

Patch

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c
index e91040af3394..ba277136f52b 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
@@ -504,8 +504,8 @@  static const struct qmp_phy_init_tbl ipq8074_pcie_serdes_tbl[] = {
 	QMP_PHY_INIT_CFG(QSERDES_COM_BG_TRIM, 0xf),
 	QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP_EN, 0x1),
 	QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_MAP, 0x0),
-	QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_TIMER1, 0x1f),
-	QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_TIMER2, 0x3f),
+	QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_TIMER1, 0xff),
+	QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_TIMER2, 0x1f),
 	QMP_PHY_INIT_CFG(QSERDES_COM_CMN_CONFIG, 0x6),
 	QMP_PHY_INIT_CFG(QSERDES_COM_PLL_IVCO, 0xf),
 	QMP_PHY_INIT_CFG(QSERDES_COM_HSCLK_SEL, 0x0),
@@ -531,7 +531,6 @@  static const struct qmp_phy_init_tbl ipq8074_pcie_serdes_tbl[] = {
 	QMP_PHY_INIT_CFG(QSERDES_COM_INTEGLOOP_GAIN1_MODE0, 0x0),
 	QMP_PHY_INIT_CFG(QSERDES_COM_INTEGLOOP_GAIN0_MODE0, 0x80),
 	QMP_PHY_INIT_CFG(QSERDES_COM_BIAS_EN_CTRL_BY_PSM, 0x1),
-	QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_CTRL, 0xa),
 	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_EN_CENTER, 0x1),
 	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_PER1, 0x31),
 	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_PER2, 0x1),
@@ -540,7 +539,6 @@  static const struct qmp_phy_init_tbl ipq8074_pcie_serdes_tbl[] = {
 	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_STEP_SIZE1, 0x2f),
 	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_STEP_SIZE2, 0x19),
 	QMP_PHY_INIT_CFG(QSERDES_COM_CLK_EP_DIV, 0x19),
-	QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_CNTRL, 0x7),
 };
 
 static const struct qmp_phy_init_tbl ipq8074_pcie_tx_tbl[] = {
@@ -548,6 +546,8 @@  static const struct qmp_phy_init_tbl ipq8074_pcie_tx_tbl[] = {
 	QMP_PHY_INIT_CFG(QSERDES_TX_LANE_MODE, 0x6),
 	QMP_PHY_INIT_CFG(QSERDES_TX_RES_CODE_LANE_OFFSET, 0x2),
 	QMP_PHY_INIT_CFG(QSERDES_TX_RCV_DETECT_LVL_2, 0x12),
+	QMP_PHY_INIT_CFG(QSERDES_TX_EMP_POST1_LVL, 0x36),
+	QMP_PHY_INIT_CFG(QSERDES_TX_SLEW_CNTL, 0x0a),
 };
 
 static const struct qmp_phy_init_tbl ipq8074_pcie_rx_tbl[] = {
@@ -558,7 +558,6 @@  static const struct qmp_phy_init_tbl ipq8074_pcie_rx_tbl[] = {
 	QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL4, 0xdb),
 	QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_SO_SATURATION_AND_ENABLE, 0x4b),
 	QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_SO_GAIN, 0x4),
-	QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_SO_GAIN_HALF, 0x4),
 };
 
 static const struct qmp_phy_init_tbl ipq8074_pcie_pcs_tbl[] = {
@@ -1673,6 +1672,9 @@  static const struct qmp_phy_cfg msm8996_usb3phy_cfg = {
 	.pwrdn_ctrl		= SW_PWRDN,
 };
 
+static const char * const ipq8074_pciephy_clk_l[] = {
+	"aux", "cfg_ahb",
+};
 /* list of resets */
 static const char * const ipq8074_pciephy_reset_l[] = {
 	"phy", "common",
@@ -1690,8 +1692,8 @@  static const struct qmp_phy_cfg ipq8074_pciephy_cfg = {
 	.rx_tbl_num		= ARRAY_SIZE(ipq8074_pcie_rx_tbl),
 	.pcs_tbl		= ipq8074_pcie_pcs_tbl,
 	.pcs_tbl_num		= ARRAY_SIZE(ipq8074_pcie_pcs_tbl),
-	.clk_list		= NULL,
-	.num_clks		= 0,
+	.clk_list		= ipq8074_pciephy_clk_l,
+	.num_clks		= ARRAY_SIZE(ipq8074_pciephy_clk_l),
 	.reset_list		= ipq8074_pciephy_reset_l,
 	.num_resets		= ARRAY_SIZE(ipq8074_pciephy_reset_l),
 	.vreg_list		= NULL,
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.h b/drivers/phy/qualcomm/phy-qcom-qmp.h
index 6d017a0c0c8d..832b3d098403 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.h
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.h
@@ -77,6 +77,8 @@ 
 #define QSERDES_COM_CORECLK_DIV_MODE1			0x1bc
 
 /* Only for QMP V2 PHY - TX registers */
+#define QSERDES_TX_EMP_POST1_LVL			0x018
+#define QSERDES_TX_SLEW_CNTL				0x040
 #define QSERDES_TX_RES_CODE_LANE_OFFSET			0x054
 #define QSERDES_TX_DEBUG_BUS_SEL			0x064
 #define QSERDES_TX_HIGHZ_TRANSCEIVEREN_BIAS_DRVR_EN	0x068