Re: [PATCH v2 6/7] phy: qualcomm: phy-qcom-qmp-ufs: Add High Speed Gear 5 support for SM8550

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Mani,

On 11/8/2023 1:49 PM, Manivannan Sadhasivam wrote:
On Tue, Nov 07, 2023 at 03:18:09PM +0200, Dmitry Baryshkov wrote:
On Tue, 7 Nov 2023 at 06:47, Can Guo <cang@xxxxxxxxxxxxxxxx> wrote:

From: Can Guo <quic_cang@xxxxxxxxxxx>

On SM8550, two sets of UFS PHY settings are provided, one set is to support
HS-G5, another set is to support HS-G4 and lower gears. The two sets of PHY
settings are programming different values to different registers, mixing
the two sets and/or overwriting one set with another set is definitely not
blessed by UFS PHY designers. In order to add HS-G5 support for SM8550, we
need to split the two sets into their dedicated tables, and leave only the
common settings in the .tlbs. To have the PHY programmed with the correct
set of PHY settings, the submode passed to PHY driver must be either HS-G4
or HS-G5.


You should also mention that this issue is also present in G4 supported targets.
And a note that it will get fixed later.

Will this info upset more people? How about I mention this in the cover letter?


Signed-off-by: Can Guo <quic_cang@xxxxxxxxxxx>
---
  drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v6.h     |   2 +
  drivers/phy/qualcomm/phy-qcom-qmp-qserdes-com-v6.h |   2 +
  .../qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v6.h    |  12 +++
  drivers/phy/qualcomm/phy-qcom-qmp-ufs.c            | 112 ++++++++++++++++++---
  4 files changed, 115 insertions(+), 13 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v6.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v6.h
index c23d5e4..e563af5 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v6.h
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v6.h
@@ -18,6 +18,7 @@
  #define QPHY_V6_PCS_UFS_BIST_FIXED_PAT_CTRL            0x060
  #define QPHY_V6_PCS_UFS_TX_HSGEAR_CAPABILITY           0x074
  #define QPHY_V6_PCS_UFS_RX_HSGEAR_CAPABILITY           0x0bc
+#define QPHY_V6_PCS_UFS_RX_HS_G5_SYNC_LENGTH_CAPABILITY        0x12c
  #define QPHY_V6_PCS_UFS_DEBUG_BUS_CLKSEL               0x158
  #define QPHY_V6_PCS_UFS_LINECFG_DISABLE                        0x17c
  #define QPHY_V6_PCS_UFS_RX_MIN_HIBERN8_TIME            0x184
@@ -27,5 +28,6 @@
  #define QPHY_V6_PCS_UFS_READY_STATUS                   0x1a8
  #define QPHY_V6_PCS_UFS_TX_MID_TERM_CTRL1              0x1f4
  #define QPHY_V6_PCS_UFS_MULTI_LANE_CTRL1               0x1fc
+#define QPHY_V6_PCS_UFS_RX_HSG5_SYNC_WAIT_TIME         0x220

  #endif
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-com-v6.h b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-com-v6.h
index f420f8f..ef392ce 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-com-v6.h
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-com-v6.h
@@ -56,6 +56,8 @@
  #define QSERDES_V6_COM_SYS_CLK_CTRL                            0xe4
  #define QSERDES_V6_COM_SYSCLK_BUF_ENABLE                       0xe8
  #define QSERDES_V6_COM_PLL_IVCO                                        0xf4
+#define QSERDES_V6_COM_CMN_IETRIM                              0xfc
+#define QSERDES_V6_COM_CMN_IPTRIM                              0x100
  #define QSERDES_V6_COM_SYSCLK_EN_SEL                           0x110
  #define QSERDES_V6_COM_RESETSM_CNTRL                           0x118
  #define QSERDES_V6_COM_LOCK_CMP_EN                             0x120
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v6.h b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v6.h
index 15bcb4b..48f31c8 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v6.h
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v6.h
@@ -10,10 +10,20 @@
  #define QSERDES_UFS_V6_TX_RES_CODE_LANE_RX                     0x2c
  #define QSERDES_UFS_V6_TX_RES_CODE_LANE_OFFSET_TX              0x30
  #define QSERDES_UFS_V6_TX_RES_CODE_LANE_OFFSET_RX              0x34
+#define QSERDES_UFS_V6_TX_LANE_MODE_1                          0x7c
+#define QSERDES_UFS_V6_TX_FR_DCC_CTRL                          0x108

  #define QSERDES_UFS_V6_RX_UCDR_FASTLOCK_FO_GAIN_RATE2          0x08
  #define QSERDES_UFS_V6_RX_UCDR_FASTLOCK_FO_GAIN_RATE4          0x10
+#define QSERDES_UFS_V6_RX_UCDR_FASTLOCK_SO_GAIN_RATE4          0x24
+#define QSERDES_UFS_V6_RX_UCDR_FASTLOCK_COUNT_HIGH_RATE4       0x54
+#define QSERDES_UFS_V6_RX_UCDR_FO_GAIN_RATE2                   0xd4
+#define QSERDES_UFS_V6_RX_UCDR_FO_GAIN_RATE4                   0xdc
+#define QSERDES_UFS_V6_RX_UCDR_SO_GAIN_RATE4                   0xf0
+#define QSERDES_UFS_V6_RX_UCDR_PI_CONTROLS                     0xf4
  #define QSERDES_UFS_V6_RX_VGA_CAL_MAN_VAL                      0x178
+#define QSERDES_UFS_V6_RX_EQ_OFFSET_ADAPTOR_CNTRL1             0x1bc
+#define QSERDES_UFS_V6_RX_OFFSET_ADAPTOR_CNTRL3                        0x1c4
  #define QSERDES_UFS_V6_RX_MODE_RATE_0_1_B0                     0x208
  #define QSERDES_UFS_V6_RX_MODE_RATE_0_1_B1                     0x20c
  #define QSERDES_UFS_V6_RX_MODE_RATE_0_1_B3                     0x214
@@ -25,6 +35,8 @@
  #define QSERDES_UFS_V6_RX_MODE_RATE3_B5                                0x264
  #define QSERDES_UFS_V6_RX_MODE_RATE3_B8                                0x270
  #define QSERDES_UFS_V6_RX_MODE_RATE4_B3                                0x280
+#define QSERDES_UFS_V6_RX_MODE_RATE4_B4                                0x284
  #define QSERDES_UFS_V6_RX_MODE_RATE4_B6                                0x28c
+#define QSERDES_UFS_V6_RX_DLL0_FTUNE_CTRL                      0x2f8

  #endif
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
index 3927eba..e0a01497 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
@@ -649,32 +649,51 @@ static const struct qmp_phy_init_tbl sm8550_ufsphy_serdes[] = {
         QMP_PHY_INIT_CFG(QSERDES_V6_COM_HSCLK_SEL_1, 0x11),
         QMP_PHY_INIT_CFG(QSERDES_V6_COM_HSCLK_HS_SWITCH_SEL_1, 0x00),
         QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP_EN, 0x01),
-       QMP_PHY_INIT_CFG(QSERDES_V6_COM_VCO_TUNE_MAP, 0x04),
-       QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_IVCO, 0x0f),
+
         QMP_PHY_INIT_CFG(QSERDES_V6_COM_VCO_TUNE_INITVAL2, 0x00),
         QMP_PHY_INIT_CFG(QSERDES_V6_COM_DEC_START_MODE0, 0x41),
-       QMP_PHY_INIT_CFG(QSERDES_V6_COM_CP_CTRL_MODE0, 0x0a),
         QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_RCTRL_MODE0, 0x18),
         QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_CCTRL_MODE0, 0x14),
         QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP1_MODE0, 0x7f),
         QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP2_MODE0, 0x06),
-       QMP_PHY_INIT_CFG(QSERDES_V6_COM_DEC_START_MODE0, 0x4c),
+};
+
+static const struct qmp_phy_init_tbl sm8550_ufsphy_hs_b_serdes[] = {
+       QMP_PHY_INIT_CFG(QSERDES_V6_COM_VCO_TUNE_MAP, 0x44),
+};
+
+static const struct qmp_phy_init_tbl sm8550_ufsphy_g4_serdes[] = {
+       QMP_PHY_INIT_CFG(QSERDES_V6_COM_VCO_TUNE_MAP, 0x04),
+       QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_IVCO, 0x0f),
         QMP_PHY_INIT_CFG(QSERDES_V6_COM_CP_CTRL_MODE0, 0x0a),
-       QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_RCTRL_MODE0, 0x18),
-       QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_CCTRL_MODE0, 0x14),
-       QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP1_MODE0, 0x99),
-       QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP2_MODE0, 0x07),

Aside from moving these registers to the HS_G4 table, you are also
changing these registers. It makes me think that there was an error in
the original programming sequence.
If that is correct, could you please split the patch into two pieces:
- Fix programming sequence (add proper Fixes tags)
- Split G4 and G5 tables.

Ack


+
+       QMP_PHY_INIT_CFG(QSERDES_V6_COM_DEC_START_MODE1, 0x4c),
+       QMP_PHY_INIT_CFG(QSERDES_V6_COM_CP_CTRL_MODE1, 0x0a),
+       QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_RCTRL_MODE1, 0x18),
+       QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_CCTRL_MODE1, 0x14),
+       QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP1_MODE1, 0x99),
+       QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP2_MODE1, 0x07),

I see all the MODE1 registers being only present in G4 and G5 tables.
Should they be programmed for the modes lower than G4?


We use G4 table for all the modes <= G4.

+};
+
+static const struct qmp_phy_init_tbl sm8550_ufsphy_g5_serdes[] = {
+       QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_IVCO, 0x1f),
+
+       QMP_PHY_INIT_CFG(QSERDES_V6_COM_CMN_IETRIM, 0x1b),
+       QMP_PHY_INIT_CFG(QSERDES_V6_COM_CMN_IPTRIM, 0x1c),
+       QMP_PHY_INIT_CFG(QSERDES_V6_COM_CP_CTRL_MODE0, 0x06),
  };

  static const struct qmp_phy_init_tbl sm8550_ufsphy_tx[] = {
-       QMP_PHY_INIT_CFG(QSERDES_V6_TX_LANE_MODE_1, 0x05),
+       QMP_PHY_INIT_CFG(QSERDES_UFS_V6_TX_LANE_MODE_1, 0x05),
         QMP_PHY_INIT_CFG(QSERDES_UFS_V6_TX_RES_CODE_LANE_OFFSET_TX, 0x07),
  };

+static const struct qmp_phy_init_tbl sm8550_ufsphy_g4_tx[] = {
+       QMP_PHY_INIT_CFG(QSERDES_UFS_V6_TX_FR_DCC_CTRL, 0x4c),
+};
+
  static const struct qmp_phy_init_tbl sm8550_ufsphy_rx[] = {
-       QMP_PHY_INIT_CFG(QSERDES_UFS_V6_RX_UCDR_FASTLOCK_FO_GAIN_RATE2, 0x0c),
-       QMP_PHY_INIT_CFG(QSERDES_UFS_V6_RX_UCDR_FASTLOCK_FO_GAIN_RATE4, 0x0f),
-       QMP_PHY_INIT_CFG(QSERDES_UFS_V6_RX_VGA_CAL_MAN_VAL, 0x0e),
+       QMP_PHY_INIT_CFG(QSERDES_UFS_V6_RX_UCDR_FO_GAIN_RATE2, 0x0c),

         QMP_PHY_INIT_CFG(QSERDES_UFS_V6_RX_MODE_RATE_0_1_B0, 0xc2),
         QMP_PHY_INIT_CFG(QSERDES_UFS_V6_RX_MODE_RATE_0_1_B1, 0xc2),
@@ -690,14 +709,46 @@ static const struct qmp_phy_init_tbl sm8550_ufsphy_rx[] = {
         QMP_PHY_INIT_CFG(QSERDES_UFS_V6_RX_MODE_RATE3_B8, 0x02),
  };

+static const struct qmp_phy_init_tbl sm8550_ufsphy_g4_rx[] = {
+       QMP_PHY_INIT_CFG(QSERDES_UFS_V6_RX_VGA_CAL_MAN_VAL, 0x0e),
+};
+
+static const struct qmp_phy_init_tbl sm8550_ufsphy_g5_rx[] = {
+       QMP_PHY_INIT_CFG(QSERDES_UFS_V6_RX_UCDR_FO_GAIN_RATE4, 0x0c),
+       QMP_PHY_INIT_CFG(QSERDES_UFS_V6_RX_UCDR_SO_GAIN_RATE4, 0x04),
+       QMP_PHY_INIT_CFG(QSERDES_UFS_V6_RX_EQ_OFFSET_ADAPTOR_CNTRL1, 0x14),
+       QMP_PHY_INIT_CFG(QSERDES_UFS_V6_RX_UCDR_PI_CONTROLS, 0x07),
+       QMP_PHY_INIT_CFG(QSERDES_UFS_V6_RX_OFFSET_ADAPTOR_CNTRL3, 0x0e),
+       QMP_PHY_INIT_CFG(QSERDES_UFS_V6_RX_UCDR_FASTLOCK_COUNT_HIGH_RATE4, 0x02),
+       QMP_PHY_INIT_CFG(QSERDES_UFS_V6_RX_UCDR_FASTLOCK_FO_GAIN_RATE4, 0x1c),
+       QMP_PHY_INIT_CFG(QSERDES_UFS_V6_RX_UCDR_FASTLOCK_SO_GAIN_RATE4, 0x06),
+       QMP_PHY_INIT_CFG(QSERDES_UFS_V6_RX_VGA_CAL_MAN_VAL, 0x08),
+
+       QMP_PHY_INIT_CFG(QSERDES_UFS_V6_RX_MODE_RATE4_B3, 0xb9),
+       QMP_PHY_INIT_CFG(QSERDES_UFS_V6_RX_MODE_RATE4_B4, 0x4f),
+       QMP_PHY_INIT_CFG(QSERDES_UFS_V6_RX_MODE_RATE4_B6, 0xff),
+       QMP_PHY_INIT_CFG(QSERDES_UFS_V6_RX_DLL0_FTUNE_CTRL, 0x30),
+};
+
  static const struct qmp_phy_init_tbl sm8550_ufsphy_pcs[] = {
         QMP_PHY_INIT_CFG(QPHY_V6_PCS_UFS_RX_SIGDET_CTRL2, 0x69),
         QMP_PHY_INIT_CFG(QPHY_V6_PCS_UFS_TX_LARGE_AMP_DRV_LVL, 0x0f),
         QMP_PHY_INIT_CFG(QPHY_V6_PCS_UFS_TX_MID_TERM_CTRL1, 0x43),
-       QMP_PHY_INIT_CFG(QPHY_V6_PCS_UFS_PLL_CNTL, 0x2b),
         QMP_PHY_INIT_CFG(QPHY_V6_PCS_UFS_MULTI_LANE_CTRL1, 0x02),
  };

+static const struct qmp_phy_init_tbl sm8550_ufsphy_g4_pcs[] = {
+       QMP_PHY_INIT_CFG(QPHY_V6_PCS_UFS_PLL_CNTL, 0x2b),
+       QMP_PHY_INIT_CFG(QPHY_V6_PCS_UFS_TX_HSGEAR_CAPABILITY, 0x04),
+       QMP_PHY_INIT_CFG(QPHY_V6_PCS_UFS_RX_HSGEAR_CAPABILITY, 0x04),
+};
+
+static const struct qmp_phy_init_tbl sm8550_ufsphy_g5_pcs[] = {
+       QMP_PHY_INIT_CFG(QPHY_V6_PCS_UFS_PLL_CNTL, 0x33),
+       QMP_PHY_INIT_CFG(QPHY_V6_PCS_UFS_RX_HS_G5_SYNC_LENGTH_CAPABILITY, 0x4f),
+       QMP_PHY_INIT_CFG(QPHY_V6_PCS_UFS_RX_HSG5_SYNC_WAIT_TIME, 0x9e),
+};
+
  struct qmp_ufs_offsets {
         u16 serdes;
         u16 pcs;
@@ -731,6 +782,8 @@ struct qmp_phy_cfg {
         const struct qmp_phy_cfg_tbls tbls_hs_b;
         /* Additional sequence for HS G4 */
         const struct qmp_phy_cfg_tbls tbls_hs_g4;
+       /* Additional sequence for HS G4 */

HS G5

Sure


+       const struct qmp_phy_cfg_tbls tbls_hs_g5;

         /* clock ids to be requested */
         const char * const *clk_list;
@@ -1157,6 +1210,28 @@ static const struct qmp_phy_cfg sm8550_ufsphy_cfg = {
                 .pcs            = sm8550_ufsphy_pcs,
                 .pcs_num        = ARRAY_SIZE(sm8550_ufsphy_pcs),
         },
+       .tbls_hs_b = {
+               .serdes         = sm8550_ufsphy_hs_b_serdes,
+               .serdes_num     = ARRAY_SIZE(sm8550_ufsphy_hs_b_serdes),
+       },
+       .tbls_hs_g4 = {
+               .serdes         = sm8550_ufsphy_g4_serdes,
+               .serdes_num     = ARRAY_SIZE(sm8550_ufsphy_g4_serdes),
+               .tx             = sm8550_ufsphy_g4_tx,
+               .tx_num         = ARRAY_SIZE(sm8550_ufsphy_g4_tx),
+               .rx             = sm8550_ufsphy_g4_rx,
+               .rx_num         = ARRAY_SIZE(sm8550_ufsphy_g4_rx),
+               .pcs            = sm8550_ufsphy_g4_pcs,
+               .pcs_num        = ARRAY_SIZE(sm8550_ufsphy_g4_pcs),
+       },
+       .tbls_hs_g5 = {
+               .serdes         = sm8550_ufsphy_g5_serdes,
+               .serdes_num     = ARRAY_SIZE(sm8550_ufsphy_g5_serdes),
+               .rx             = sm8550_ufsphy_g5_rx,
+               .rx_num         = ARRAY_SIZE(sm8550_ufsphy_g5_rx),
+               .pcs            = sm8550_ufsphy_g5_pcs,
+               .pcs_num        = ARRAY_SIZE(sm8550_ufsphy_g5_pcs),
+       },
         .clk_list               = sdm845_ufs_phy_clk_l,
         .num_clks               = ARRAY_SIZE(sdm845_ufs_phy_clk_l),
         .vreg_list              = qmp_phy_vreg_l,
@@ -1222,14 +1297,25 @@ static void qmp_ufs_pcs_init(struct qmp_ufs *qmp, const struct qmp_phy_cfg_tbls
  static void qmp_ufs_init_registers(struct qmp_ufs *qmp, const struct qmp_phy_cfg *cfg)
  {
         qmp_ufs_serdes_init(qmp, &cfg->tbls);
+       if (qmp->submode == UFS_HS_G4)
+               qmp_ufs_serdes_init(qmp, &cfg->tbls_hs_g4);
+       else if (qmp->submode == UFS_HS_G5)
+               qmp_ufs_serdes_init(qmp, &cfg->tbls_hs_g5);
+

Should we program submode sequence after HS_B?

- Mani

As the UFS PHY HW programming doc mentions, order of these writes is not important, so long as they are programmed correctly before release S/W reset, then it is fine.

Thanks,
Can Guo.






[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux