* Re: [PATCH v5 09/10] phy: qualcomm: phy-qcom-qmp-ufs: Add High Speed Gear 5 support for SM8550
2023-11-23 8:46 ` [PATCH v5 09/10] phy: qualcomm: phy-qcom-qmp-ufs: Add High Speed Gear 5 support for SM8550 Can Guo
@ 2023-11-23 12:35 ` Dmitry Baryshkov
2023-11-24 1:55 ` Can Guo
2023-11-28 6:47 ` Manivannan Sadhasivam
2023-11-28 9:59 ` neil.armstrong
2 siblings, 1 reply; 11+ messages in thread
From: Dmitry Baryshkov @ 2023-11-23 12:35 UTC (permalink / raw)
To: Can Guo
Cc: bvanassche, mani, adrian.hunter, beanhuo, avri.altman,
junwoo80.lee, martin.petersen, linux-scsi, linux-arm-msm,
Andy Gross, Bjorn Andersson, Konrad Dybcio, Vinod Koul,
Kishon Vijay Abraham I, open list:GENERIC PHY FRAMEWORK,
open list
On Thu, 23 Nov 2023 at 10:47, Can Guo <quic_cang@quicinc.com> wrote:
>
> 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.
>
> To add HS-G5 support for SM8550, split the two sets of PHY settings into
> their dedicated overlay tables, only the common parts of the two sets of
> PHY settings are left in the .tbls.
>
> Consider we are going to add even higher gear support in future, to avoid
> adding more tables with different names, rename the .tbls_hs_g4 and make it
> an array, a size of 2 is enough as of now.
>
> In this case, .tbls alone is not a complete set of PHY settings, so either
> tbls_hs_overlay[0] or tbls_hs_overlay[1] must be applied on top of the
> .tbls to become a complete set of PHY settings.
>
> Signed-off-by: Can Guo <quic_cang@quicinc.com>
> ---
> 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 | 9 ++
> drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 174 ++++++++++++++++++---
> 4 files changed, 166 insertions(+), 21 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 674f158..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
> @@ -15,8 +15,15 @@
>
> #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
> @@ -28,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 ad91f92..29106ec 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> @@ -41,6 +41,8 @@
>
> #define PHY_INIT_COMPLETE_TIMEOUT 10000
>
> +#define NUM_OVERLAY 2
> +
> struct qmp_phy_init_tbl {
> unsigned int offset;
> unsigned int val;
> @@ -649,15 +651,22 @@ 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),
> +};
> +
> +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_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),
> @@ -666,19 +675,24 @@ static const struct qmp_phy_init_tbl sm8550_ufsphy_serdes[] = {
> QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP2_MODE1, 0x07),
> };
>
> -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_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_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_FO_GAIN_RATE2, 0x0c),
> - QMP_PHY_INIT_CFG(QSERDES_UFS_V6_RX_VGA_CAL_MAN_VAL, 0x0e),
>
> 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),
> @@ -694,16 +708,45 @@ 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;
> @@ -723,6 +766,8 @@ struct qmp_phy_cfg_tbls {
> int rx_num;
> const struct qmp_phy_init_tbl *pcs;
> int pcs_num;
> + /* Maximum supported Gear of this tbls */
> + u32 max_gear;
> };
>
> /* struct qmp_phy_cfg - per-PHY initialization config */
> @@ -730,13 +775,15 @@ struct qmp_phy_cfg {
> int lanes;
>
> const struct qmp_ufs_offsets *offsets;
> + /* Maximum supported Gear of this config */
> + u32 max_supported_gear;
>
> /* Main init sequence for PHY blocks - serdes, tx, rx, pcs */
> const struct qmp_phy_cfg_tbls tbls;
> /* Additional sequence for HS Series B */
> 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 different HS Gears */
> + const struct qmp_phy_cfg_tbls tbls_hs_overlay[NUM_OVERLAY];
>
> /* clock ids to be requested */
> const char * const *clk_list;
> @@ -839,6 +886,7 @@ static const struct qmp_phy_cfg msm8996_ufsphy_cfg = {
> .lanes = 1,
>
> .offsets = &qmp_ufs_offsets,
> + .max_supported_gear = UFS_HS_G3,
>
> .tbls = {
> .serdes = msm8996_ufsphy_serdes,
> @@ -864,6 +912,7 @@ static const struct qmp_phy_cfg sa8775p_ufsphy_cfg = {
> .lanes = 2,
>
> .offsets = &qmp_ufs_offsets,
> + .max_supported_gear = UFS_HS_G4,
>
> .tbls = {
> .serdes = sm8350_ufsphy_serdes,
> @@ -879,13 +928,14 @@ static const struct qmp_phy_cfg sa8775p_ufsphy_cfg = {
> .serdes = sm8350_ufsphy_hs_b_serdes,
> .serdes_num = ARRAY_SIZE(sm8350_ufsphy_hs_b_serdes),
> },
> - .tbls_hs_g4 = {
> + .tbls_hs_overlay[0] = {
> .tx = sm8350_ufsphy_g4_tx,
> .tx_num = ARRAY_SIZE(sm8350_ufsphy_g4_tx),
> .rx = sm8350_ufsphy_g4_rx,
> .rx_num = ARRAY_SIZE(sm8350_ufsphy_g4_rx),
> .pcs = sm8350_ufsphy_g4_pcs,
> .pcs_num = ARRAY_SIZE(sm8350_ufsphy_g4_pcs),
> + .max_gear = UFS_HS_G4,
> },
> .clk_list = sm8450_ufs_phy_clk_l,
> .num_clks = ARRAY_SIZE(sm8450_ufs_phy_clk_l),
> @@ -898,6 +948,7 @@ static const struct qmp_phy_cfg sc8280xp_ufsphy_cfg = {
> .lanes = 2,
>
> .offsets = &qmp_ufs_offsets,
> + .max_supported_gear = UFS_HS_G4,
>
> .tbls = {
> .serdes = sm8350_ufsphy_serdes,
> @@ -913,13 +964,14 @@ static const struct qmp_phy_cfg sc8280xp_ufsphy_cfg = {
> .serdes = sm8350_ufsphy_hs_b_serdes,
> .serdes_num = ARRAY_SIZE(sm8350_ufsphy_hs_b_serdes),
> },
> - .tbls_hs_g4 = {
> + .tbls_hs_overlay[0] = {
> .tx = sm8350_ufsphy_g4_tx,
> .tx_num = ARRAY_SIZE(sm8350_ufsphy_g4_tx),
> .rx = sm8350_ufsphy_g4_rx,
> .rx_num = ARRAY_SIZE(sm8350_ufsphy_g4_rx),
> .pcs = sm8350_ufsphy_g4_pcs,
> .pcs_num = ARRAY_SIZE(sm8350_ufsphy_g4_pcs),
> + .max_gear = UFS_HS_G4,
> },
> .clk_list = sdm845_ufs_phy_clk_l,
> .num_clks = ARRAY_SIZE(sdm845_ufs_phy_clk_l),
> @@ -932,6 +984,7 @@ static const struct qmp_phy_cfg sdm845_ufsphy_cfg = {
> .lanes = 2,
>
> .offsets = &qmp_ufs_offsets,
> + .max_supported_gear = UFS_HS_G3,
>
> .tbls = {
> .serdes = sdm845_ufsphy_serdes,
> @@ -960,6 +1013,7 @@ static const struct qmp_phy_cfg sm6115_ufsphy_cfg = {
> .lanes = 1,
>
> .offsets = &qmp_ufs_offsets,
> + .max_supported_gear = UFS_HS_G3,
>
> .tbls = {
> .serdes = sm6115_ufsphy_serdes,
> @@ -988,6 +1042,7 @@ static const struct qmp_phy_cfg sm7150_ufsphy_cfg = {
> .lanes = 1,
>
> .offsets = &qmp_ufs_offsets,
> + .max_supported_gear = UFS_HS_G3,
>
> .tbls = {
> .serdes = sdm845_ufsphy_serdes,
> @@ -1016,6 +1071,7 @@ static const struct qmp_phy_cfg sm8150_ufsphy_cfg = {
> .lanes = 2,
>
> .offsets = &qmp_ufs_offsets,
> + .max_supported_gear = UFS_HS_G4,
>
> .tbls = {
> .serdes = sm8150_ufsphy_serdes,
> @@ -1031,13 +1087,14 @@ static const struct qmp_phy_cfg sm8150_ufsphy_cfg = {
> .serdes = sm8150_ufsphy_hs_b_serdes,
> .serdes_num = ARRAY_SIZE(sm8150_ufsphy_hs_b_serdes),
> },
> - .tbls_hs_g4 = {
> + .tbls_hs_overlay[0] = {
> .tx = sm8150_ufsphy_hs_g4_tx,
> .tx_num = ARRAY_SIZE(sm8150_ufsphy_hs_g4_tx),
> .rx = sm8150_ufsphy_hs_g4_rx,
> .rx_num = ARRAY_SIZE(sm8150_ufsphy_hs_g4_rx),
> .pcs = sm8150_ufsphy_hs_g4_pcs,
> .pcs_num = ARRAY_SIZE(sm8150_ufsphy_hs_g4_pcs),
> + .max_gear = UFS_HS_G4,
> },
> .clk_list = sdm845_ufs_phy_clk_l,
> .num_clks = ARRAY_SIZE(sdm845_ufs_phy_clk_l),
> @@ -1050,6 +1107,7 @@ static const struct qmp_phy_cfg sm8250_ufsphy_cfg = {
> .lanes = 2,
>
> .offsets = &qmp_ufs_offsets,
> + .max_supported_gear = UFS_HS_G4,
>
> .tbls = {
> .serdes = sm8150_ufsphy_serdes,
> @@ -1065,13 +1123,14 @@ static const struct qmp_phy_cfg sm8250_ufsphy_cfg = {
> .serdes = sm8150_ufsphy_hs_b_serdes,
> .serdes_num = ARRAY_SIZE(sm8150_ufsphy_hs_b_serdes),
> },
> - .tbls_hs_g4 = {
> + .tbls_hs_overlay[0] = {
> .tx = sm8250_ufsphy_hs_g4_tx,
> .tx_num = ARRAY_SIZE(sm8250_ufsphy_hs_g4_tx),
> .rx = sm8250_ufsphy_hs_g4_rx,
> .rx_num = ARRAY_SIZE(sm8250_ufsphy_hs_g4_rx),
> .pcs = sm8150_ufsphy_hs_g4_pcs,
> .pcs_num = ARRAY_SIZE(sm8150_ufsphy_hs_g4_pcs),
> + .max_gear = UFS_HS_G4,
> },
> .clk_list = sdm845_ufs_phy_clk_l,
> .num_clks = ARRAY_SIZE(sdm845_ufs_phy_clk_l),
> @@ -1084,6 +1143,7 @@ static const struct qmp_phy_cfg sm8350_ufsphy_cfg = {
> .lanes = 2,
>
> .offsets = &qmp_ufs_offsets,
> + .max_supported_gear = UFS_HS_G4,
>
> .tbls = {
> .serdes = sm8350_ufsphy_serdes,
> @@ -1099,13 +1159,14 @@ static const struct qmp_phy_cfg sm8350_ufsphy_cfg = {
> .serdes = sm8350_ufsphy_hs_b_serdes,
> .serdes_num = ARRAY_SIZE(sm8350_ufsphy_hs_b_serdes),
> },
> - .tbls_hs_g4 = {
> + .tbls_hs_overlay[0] = {
> .tx = sm8350_ufsphy_g4_tx,
> .tx_num = ARRAY_SIZE(sm8350_ufsphy_g4_tx),
> .rx = sm8350_ufsphy_g4_rx,
> .rx_num = ARRAY_SIZE(sm8350_ufsphy_g4_rx),
> .pcs = sm8350_ufsphy_g4_pcs,
> .pcs_num = ARRAY_SIZE(sm8350_ufsphy_g4_pcs),
> + .max_gear = UFS_HS_G4,
> },
> .clk_list = sdm845_ufs_phy_clk_l,
> .num_clks = ARRAY_SIZE(sdm845_ufs_phy_clk_l),
> @@ -1118,6 +1179,7 @@ static const struct qmp_phy_cfg sm8450_ufsphy_cfg = {
> .lanes = 2,
>
> .offsets = &qmp_ufs_offsets,
> + .max_supported_gear = UFS_HS_G4,
>
> .tbls = {
> .serdes = sm8350_ufsphy_serdes,
> @@ -1133,13 +1195,14 @@ static const struct qmp_phy_cfg sm8450_ufsphy_cfg = {
> .serdes = sm8350_ufsphy_hs_b_serdes,
> .serdes_num = ARRAY_SIZE(sm8350_ufsphy_hs_b_serdes),
> },
> - .tbls_hs_g4 = {
> + .tbls_hs_overlay[0] = {
> .tx = sm8350_ufsphy_g4_tx,
> .tx_num = ARRAY_SIZE(sm8350_ufsphy_g4_tx),
> .rx = sm8350_ufsphy_g4_rx,
> .rx_num = ARRAY_SIZE(sm8350_ufsphy_g4_rx),
> .pcs = sm8350_ufsphy_g4_pcs,
> .pcs_num = ARRAY_SIZE(sm8350_ufsphy_g4_pcs),
> + .max_gear = UFS_HS_G4,
> },
> .clk_list = sm8450_ufs_phy_clk_l,
> .num_clks = ARRAY_SIZE(sm8450_ufs_phy_clk_l),
> @@ -1152,6 +1215,7 @@ static const struct qmp_phy_cfg sm8550_ufsphy_cfg = {
> .lanes = 2,
>
> .offsets = &qmp_ufs_offsets_v6,
> + .max_supported_gear = UFS_HS_G5,
>
> .tbls = {
> .serdes = sm8550_ufsphy_serdes,
> @@ -1167,6 +1231,26 @@ static const struct qmp_phy_cfg sm8550_ufsphy_cfg = {
> .serdes = sm8550_ufsphy_hs_b_serdes,
> .serdes_num = ARRAY_SIZE(sm8550_ufsphy_hs_b_serdes),
> },
> + .tbls_hs_overlay[0] = {
> + .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),
> + .max_gear = UFS_HS_G4,
> + },
> + .tbls_hs_overlay[1] = {
> + .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),
> + .max_gear = UFS_HS_G5,
> + },
> .clk_list = sdm845_ufs_phy_clk_l,
> .num_clks = ARRAY_SIZE(sdm845_ufs_phy_clk_l),
> .vreg_list = qmp_phy_vreg_l,
> @@ -1229,17 +1313,63 @@ static void qmp_ufs_pcs_init(struct qmp_ufs *qmp, const struct qmp_phy_cfg_tbls
> qmp_ufs_configure(pcs, tbls->pcs, tbls->pcs_num);
> }
>
> -static void qmp_ufs_init_registers(struct qmp_ufs *qmp, const struct qmp_phy_cfg *cfg)
> +static bool qmp_ufs_match_gear_overlay(struct qmp_ufs *qmp, const struct qmp_phy_cfg *cfg, int *i)
You can simply return int from this function. -EINVAL would mean that
the setting was not found. Also this can make max_supported_gear
unused.
> +{
> + u32 max_gear, floor_max_gear = cfg->max_supported_gear;
> + bool found = false;
> + int j;
> +
> + for (j = 0; j < NUM_OVERLAY; j ++) {
> + max_gear = cfg->tbls_hs_overlay[j].max_gear;
> +
> + if (max_gear == 0)
> + continue;
> +
> + /* Direct matching, bail */
> + if (qmp->submode == max_gear) {
> + *i = j;
> + return true;
> + }
> +
> + /* If no direct matching, the lowest gear is the best matching */
> + if (max_gear < floor_max_gear) {
> + *i = j;
> + found = true;
> + floor_max_gear = max_gear;
> + }
We know that the table is sorted. So we can return an index of the
first setting that fits.
> + }
> +
> + return found;
> +}
> +
> +static int qmp_ufs_init_registers(struct qmp_ufs *qmp, const struct qmp_phy_cfg *cfg)
> {
> + bool apply_overlay;
> + int i;
> +
> + if (qmp->submode > cfg->max_supported_gear || qmp->submode == 0) {
> + dev_err(qmp->dev, "Invalid PHY submode %u\n", qmp->submode);
> + return -EINVAL;
> + }
> +
> + apply_overlay = qmp_ufs_match_gear_overlay(qmp, cfg, &i);
> +
> qmp_ufs_serdes_init(qmp, &cfg->tbls);
> + if (apply_overlay)
> + qmp_ufs_serdes_init(qmp, &cfg->tbls_hs_overlay[i]);
> +
> if (qmp->mode == PHY_MODE_UFS_HS_B)
> qmp_ufs_serdes_init(qmp, &cfg->tbls_hs_b);
> +
> qmp_ufs_lanes_init(qmp, &cfg->tbls);
> - if (qmp->submode == UFS_HS_G4)
> - qmp_ufs_lanes_init(qmp, &cfg->tbls_hs_g4);
> + if (apply_overlay)
> + qmp_ufs_lanes_init(qmp, &cfg->tbls_hs_overlay[i]);
> +
> qmp_ufs_pcs_init(qmp, &cfg->tbls);
> - if (qmp->submode == UFS_HS_G4)
> - qmp_ufs_pcs_init(qmp, &cfg->tbls_hs_g4);
> + if (apply_overlay)
> + qmp_ufs_pcs_init(qmp, &cfg->tbls_hs_overlay[i]);
> +
> + return 0;
> }
>
> static int qmp_ufs_com_init(struct qmp_ufs *qmp)
> @@ -1331,7 +1461,9 @@ static int qmp_ufs_power_on(struct phy *phy)
> unsigned int val;
> int ret;
>
> - qmp_ufs_init_registers(qmp, cfg);
> + ret = qmp_ufs_init_registers(qmp, cfg);
> + if (ret)
> + return ret;
>
> ret = reset_control_deassert(qmp->ufs_reset);
> if (ret)
> --
> 2.7.4
>
>
--
With best wishes
Dmitry
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v5 09/10] phy: qualcomm: phy-qcom-qmp-ufs: Add High Speed Gear 5 support for SM8550
2023-11-23 12:35 ` Dmitry Baryshkov
@ 2023-11-24 1:55 ` Can Guo
0 siblings, 0 replies; 11+ messages in thread
From: Can Guo @ 2023-11-24 1:55 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: bvanassche, mani, adrian.hunter, beanhuo, avri.altman,
junwoo80.lee, martin.petersen, linux-scsi, linux-arm-msm,
Andy Gross, Bjorn Andersson, Konrad Dybcio, Vinod Koul,
Kishon Vijay Abraham I, open list:GENERIC PHY FRAMEWORK,
open list
On 11/23/2023 8:35 PM, Dmitry Baryshkov wrote:
> On Thu, 23 Nov 2023 at 10:47, Can Guo <quic_cang@quicinc.com> wrote:
>>
>> 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.
>>
>> To add HS-G5 support for SM8550, split the two sets of PHY settings into
>> their dedicated overlay tables, only the common parts of the two sets of
>> PHY settings are left in the .tbls.
>>
>> Consider we are going to add even higher gear support in future, to avoid
>> adding more tables with different names, rename the .tbls_hs_g4 and make it
>> an array, a size of 2 is enough as of now.
>>
>> In this case, .tbls alone is not a complete set of PHY settings, so either
>> tbls_hs_overlay[0] or tbls_hs_overlay[1] must be applied on top of the
>> .tbls to become a complete set of PHY settings.
>>
>> Signed-off-by: Can Guo <quic_cang@quicinc.com>
>>
>> -static void qmp_ufs_init_registers(struct qmp_ufs *qmp, const struct qmp_phy_cfg *cfg)
>> +static bool qmp_ufs_match_gear_overlay(struct qmp_ufs *qmp, const struct qmp_phy_cfg *cfg, int *i)
>
> You can simply return int from this function. -EINVAL would mean that
> the setting was not found. Also this can make max_supported_gear
> unused.
I will return int in next version, but I'd like to keep the
max_support_gear, because for platforms which only .tbls is provided (no
overlay case), we need the max_supported_gear to tell whether the
requested submode is exceeding the capability provided by the PHY settings.
>
>> +{
>> + u32 max_gear, floor_max_gear = cfg->max_supported_gear;
>> + bool found = false;
>> + int j;
>> +
>> + for (j = 0; j < NUM_OVERLAY; j ++) {
>> + max_gear = cfg->tbls_hs_overlay[j].max_gear;
>> +
>> + if (max_gear == 0)
>> + continue;
>> +
>> + /* Direct matching, bail */
>> + if (qmp->submode == max_gear) {
>> + *i = j;
>> + return true;
>> + }
>> +
>> + /* If no direct matching, the lowest gear is the best matching */
>> + if (max_gear < floor_max_gear) {
>> + *i = j;
>> + found = true;
>> + floor_max_gear = max_gear;
>> + }
>
> We know that the table is sorted. So we can return an index of the
> first setting that fits.
For SM8550, it is OK, because no-G5 settings are in overlay[0] and G5
settings are in overlay[1], applying one overlay is a must.
.tbls | support nothing as it is incomplete
.tbls + .tbls_hs_overlay[0] | support G4 and lower gears
.tb.s + .tbls_hs_overlay[1] | support G5
But for previously added platforms, no. I put it this way for two reasons -
1. In case the tables are not sorted.
2. For previously added targets, whose configs support G4 and no-G4:
.tbls | support G3 and lower gears
.tbls + .tbls_hs_overlay | support G4
if we anways return an index of the first setting that fits, for these
targets, the G4 settings would always be programmed, no matter UFS
driver requests for G2/G3/G4. On these targets, as dual UFS init is
there to find the most power saving PHY settings, when UFS driver
requests for G2/G3, .tbls_hs_overlay should NOT be applied. Otherwise,
it defeats all the efforts which Mani had spent for the dual UFS init.
Thanks,
Can Guo.
>
>> + }
>> +
>> + return found;
>> +}
>> +
>> +static int qmp_ufs_init_registers(struct qmp_ufs *qmp, const struct qmp_phy_cfg *cfg)
>> {
>> + bool apply_overlay;
>> + int i;
>> +
>> + if (qmp->submode > cfg->max_supported_gear || qmp->submode == 0) {
>> + dev_err(qmp->dev, "Invalid PHY submode %u\n", qmp->submode);
>> + return -EINVAL;
>> + }
>> +
>> + apply_overlay = qmp_ufs_match_gear_overlay(qmp, cfg, &i);
>> +
>> qmp_ufs_serdes_init(qmp, &cfg->tbls);
>> + if (apply_overlay)
>> + qmp_ufs_serdes_init(qmp, &cfg->tbls_hs_overlay[i]);
>> +
>> if (qmp->mode == PHY_MODE_UFS_HS_B)
>> qmp_ufs_serdes_init(qmp, &cfg->tbls_hs_b);
>> +
>> qmp_ufs_lanes_init(qmp, &cfg->tbls);
>> - if (qmp->submode == UFS_HS_G4)
>> - qmp_ufs_lanes_init(qmp, &cfg->tbls_hs_g4);
>> + if (apply_overlay)
>> + qmp_ufs_lanes_init(qmp, &cfg->tbls_hs_overlay[i]);
>> +
>> qmp_ufs_pcs_init(qmp, &cfg->tbls);
>> - if (qmp->submode == UFS_HS_G4)
>> - qmp_ufs_pcs_init(qmp, &cfg->tbls_hs_g4);
>> + if (apply_overlay)
>> + qmp_ufs_pcs_init(qmp, &cfg->tbls_hs_overlay[i]);
>> +
>> + return 0;
>> }
>>
>> static int qmp_ufs_com_init(struct qmp_ufs *qmp)
>> @@ -1331,7 +1461,9 @@ static int qmp_ufs_power_on(struct phy *phy)
>> unsigned int val;
>> int ret;
>>
>> - qmp_ufs_init_registers(qmp, cfg);
>> + ret = qmp_ufs_init_registers(qmp, cfg);
>> + if (ret)
>> + return ret;
>>
>> ret = reset_control_deassert(qmp->ufs_reset);
>> if (ret)
>> --
>> 2.7.4
>>
>>
>
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 09/10] phy: qualcomm: phy-qcom-qmp-ufs: Add High Speed Gear 5 support for SM8550
2023-11-23 8:46 ` [PATCH v5 09/10] phy: qualcomm: phy-qcom-qmp-ufs: Add High Speed Gear 5 support for SM8550 Can Guo
2023-11-23 12:35 ` Dmitry Baryshkov
@ 2023-11-28 6:47 ` Manivannan Sadhasivam
2023-11-28 9:00 ` Can Guo
2023-11-28 9:59 ` neil.armstrong
2 siblings, 1 reply; 11+ messages in thread
From: Manivannan Sadhasivam @ 2023-11-28 6:47 UTC (permalink / raw)
To: Can Guo
Cc: bvanassche, adrian.hunter, beanhuo, avri.altman, junwoo80.lee,
martin.petersen, linux-scsi, linux-arm-msm, Andy Gross,
Bjorn Andersson, Konrad Dybcio, Vinod Koul,
Kishon Vijay Abraham I, open list:GENERIC PHY FRAMEWORK,
open list
On Thu, Nov 23, 2023 at 12:46:29AM -0800, Can Guo wrote:
> 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.
>
> To add HS-G5 support for SM8550, split the two sets of PHY settings into
> their dedicated overlay tables, only the common parts of the two sets of
> PHY settings are left in the .tbls.
>
> Consider we are going to add even higher gear support in future, to avoid
> adding more tables with different names, rename the .tbls_hs_g4 and make it
> an array, a size of 2 is enough as of now.
>
> In this case, .tbls alone is not a complete set of PHY settings, so either
> tbls_hs_overlay[0] or tbls_hs_overlay[1] must be applied on top of the
> .tbls to become a complete set of PHY settings.
>
Thanks for the update! This really helps in minimizing the changes for future
gears.
> Signed-off-by: Can Guo <quic_cang@quicinc.com>
> ---
> 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 | 9 ++
> drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 174 ++++++++++++++++++---
> 4 files changed, 166 insertions(+), 21 deletions(-)
>
>
[...]
> -static void qmp_ufs_init_registers(struct qmp_ufs *qmp, const struct qmp_phy_cfg *cfg)
> +static bool qmp_ufs_match_gear_overlay(struct qmp_ufs *qmp, const struct qmp_phy_cfg *cfg, int *i)
> +{
> + u32 max_gear, floor_max_gear = cfg->max_supported_gear;
> + bool found = false;
> + int j;
> +
> + for (j = 0; j < NUM_OVERLAY; j ++) {
> + max_gear = cfg->tbls_hs_overlay[j].max_gear;
> +
> + if (max_gear == 0)
Is this condition possible for hs_overlay tables?
> + continue;
> +
> + /* Direct matching, bail */
> + if (qmp->submode == max_gear) {
> + *i = j;
> + return true;
> + }
> +
> + /* If no direct matching, the lowest gear is the best matching */
> + if (max_gear < floor_max_gear) {
Can you start the loop from max? If looks odd to set the matching params in the
first iteration itself and then checking the next one.
> + *i = j;
> + found = true;
> + floor_max_gear = max_gear;
> + }
> + }
> +
> + return found;
> +}
> +
> +static int qmp_ufs_init_registers(struct qmp_ufs *qmp, const struct qmp_phy_cfg *cfg)
> {
> + bool apply_overlay;
> + int i;
> +
> + if (qmp->submode > cfg->max_supported_gear || qmp->submode == 0) {
> + dev_err(qmp->dev, "Invalid PHY submode %u\n", qmp->submode);
> + return -EINVAL;
> + }
This check should be moved to qmp_ufs_set_mode().
Rest LGTM.
- Mani
--
மணிவண்ணன் சதாசிவம்
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v5 09/10] phy: qualcomm: phy-qcom-qmp-ufs: Add High Speed Gear 5 support for SM8550
2023-11-28 6:47 ` Manivannan Sadhasivam
@ 2023-11-28 9:00 ` Can Guo
0 siblings, 0 replies; 11+ messages in thread
From: Can Guo @ 2023-11-28 9:00 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: bvanassche, adrian.hunter, beanhuo, avri.altman, junwoo80.lee,
martin.petersen, linux-scsi, linux-arm-msm, Andy Gross,
Bjorn Andersson, Konrad Dybcio, Vinod Koul,
Kishon Vijay Abraham I, open list:GENERIC PHY FRAMEWORK,
open list
On 11/28/2023 2:47 PM, Manivannan Sadhasivam wrote:
> On Thu, Nov 23, 2023 at 12:46:29AM -0800, Can Guo wrote:
>> 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.
>>
>> To add HS-G5 support for SM8550, split the two sets of PHY settings into
>> their dedicated overlay tables, only the common parts of the two sets of
>> PHY settings are left in the .tbls.
>>
>> Consider we are going to add even higher gear support in future, to avoid
>> adding more tables with different names, rename the .tbls_hs_g4 and make it
>> an array, a size of 2 is enough as of now.
>>
>> In this case, .tbls alone is not a complete set of PHY settings, so either
>> tbls_hs_overlay[0] or tbls_hs_overlay[1] must be applied on top of the
>> .tbls to become a complete set of PHY settings.
>>
>
> Thanks for the update! This really helps in minimizing the changes for future
> gears.
>
>> Signed-off-by: Can Guo <quic_cang@quicinc.com>
>> ---
>> 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 | 9 ++
>> drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 174 ++++++++++++++++++---
>> 4 files changed, 166 insertions(+), 21 deletions(-)
>>
>>
>
> [...]
>
>> -static void qmp_ufs_init_registers(struct qmp_ufs *qmp, const struct qmp_phy_cfg *cfg)
>> +static bool qmp_ufs_match_gear_overlay(struct qmp_ufs *qmp, const struct qmp_phy_cfg *cfg, int *i)
>> +{
>> + u32 max_gear, floor_max_gear = cfg->max_supported_gear;
>> + bool found = false;
>> + int j;
>> +
>> + for (j = 0; j < NUM_OVERLAY; j ++) {
>> + max_gear = cfg->tbls_hs_overlay[j].max_gear;
>> +
>> + if (max_gear == 0)
>
> Is this condition possible for hs_overlay tables?
Yes, now there are 2 overlays for SM8550, but only one overlay for the
rest targets. For those who has only one overlay, this check fits them.
>
>> + continue;
>> +
>> + /* Direct matching, bail */
>> + if (qmp->submode == max_gear) {
>> + *i = j;
>> + return true;
>> + }
>> +
>> + /* If no direct matching, the lowest gear is the best matching */
>> + if (max_gear < floor_max_gear) {
>
> Can you start the loop from max? If looks odd to set the matching params in the
> first iteration itself and then checking the next one.
OK, will start from j = NUM_VERLAY - 1; in next version. When I wrote
the code, I was not expecting the max is always in the last overlay,
they can come in any orders.
>
>> + *i = j;
>> + found = true;
>> + floor_max_gear = max_gear;
>> + }
>> + }
>> +
>> + return found;
>> +}
>> +
>> +static int qmp_ufs_init_registers(struct qmp_ufs *qmp, const struct qmp_phy_cfg *cfg)
>> {
>> + bool apply_overlay;
>> + int i;
>> +
>> + if (qmp->submode > cfg->max_supported_gear || qmp->submode == 0) {
>> + dev_err(qmp->dev, "Invalid PHY submode %u\n", qmp->submode);
>> + return -EINVAL;
>> + }
>
> This check should be moved to qmp_ufs_set_mode().
OK.
Thanks,
Can Guo.
>
> Rest LGTM.
>
> - Mani
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 09/10] phy: qualcomm: phy-qcom-qmp-ufs: Add High Speed Gear 5 support for SM8550
2023-11-23 8:46 ` [PATCH v5 09/10] phy: qualcomm: phy-qcom-qmp-ufs: Add High Speed Gear 5 support for SM8550 Can Guo
2023-11-23 12:35 ` Dmitry Baryshkov
2023-11-28 6:47 ` Manivannan Sadhasivam
@ 2023-11-28 9:59 ` neil.armstrong
2023-11-28 10:03 ` Can Guo
2 siblings, 1 reply; 11+ messages in thread
From: neil.armstrong @ 2023-11-28 9:59 UTC (permalink / raw)
To: Can Guo, bvanassche, mani, adrian.hunter, beanhuo, avri.altman,
junwoo80.lee, martin.petersen
Cc: linux-scsi, linux-arm-msm, Andy Gross, Bjorn Andersson,
Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I,
open list:GENERIC PHY FRAMEWORK, open list
Hi,
On 23/11/2023 09:46, Can Guo wrote:
> 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.
>
> To add HS-G5 support for SM8550, split the two sets of PHY settings into
> their dedicated overlay tables, only the common parts of the two sets of
> PHY settings are left in the .tbls.
>
> Consider we are going to add even higher gear support in future, to avoid
> adding more tables with different names, rename the .tbls_hs_g4 and make it
> an array, a size of 2 is enough as of now.
>
> In this case, .tbls alone is not a complete set of PHY settings, so either
> tbls_hs_overlay[0] or tbls_hs_overlay[1] must be applied on top of the
> .tbls to become a complete set of PHY settings.
>
> Signed-off-by: Can Guo <quic_cang@quicinc.com>
Since Vinod has already merged SM8650 UFS PHY support, I'm afraid this
serie will break UFS if the SM8650 UFS tables aren't updated aswell.
Could you confirm if this will be the case ?
Thanks,
Neil
> ---
> 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 | 9 ++
> drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 174 ++++++++++++++++++---
> 4 files changed, 166 insertions(+), 21 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 674f158..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
> @@ -15,8 +15,15 @@
>
> #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
> @@ -28,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 ad91f92..29106ec 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c
> @@ -41,6 +41,8 @@
>
> #define PHY_INIT_COMPLETE_TIMEOUT 10000
>
> +#define NUM_OVERLAY 2
> +
> struct qmp_phy_init_tbl {
> unsigned int offset;
> unsigned int val;
> @@ -649,15 +651,22 @@ 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),
> +};
> +
> +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_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),
> @@ -666,19 +675,24 @@ static const struct qmp_phy_init_tbl sm8550_ufsphy_serdes[] = {
> QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP2_MODE1, 0x07),
> };
>
> -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_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_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_FO_GAIN_RATE2, 0x0c),
> - QMP_PHY_INIT_CFG(QSERDES_UFS_V6_RX_VGA_CAL_MAN_VAL, 0x0e),
>
> 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),
> @@ -694,16 +708,45 @@ 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;
> @@ -723,6 +766,8 @@ struct qmp_phy_cfg_tbls {
> int rx_num;
> const struct qmp_phy_init_tbl *pcs;
> int pcs_num;
> + /* Maximum supported Gear of this tbls */
> + u32 max_gear;
> };
>
> /* struct qmp_phy_cfg - per-PHY initialization config */
> @@ -730,13 +775,15 @@ struct qmp_phy_cfg {
> int lanes;
>
> const struct qmp_ufs_offsets *offsets;
> + /* Maximum supported Gear of this config */
> + u32 max_supported_gear;
>
> /* Main init sequence for PHY blocks - serdes, tx, rx, pcs */
> const struct qmp_phy_cfg_tbls tbls;
> /* Additional sequence for HS Series B */
> 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 different HS Gears */
> + const struct qmp_phy_cfg_tbls tbls_hs_overlay[NUM_OVERLAY];
>
> /* clock ids to be requested */
> const char * const *clk_list;
> @@ -839,6 +886,7 @@ static const struct qmp_phy_cfg msm8996_ufsphy_cfg = {
> .lanes = 1,
>
> .offsets = &qmp_ufs_offsets,
> + .max_supported_gear = UFS_HS_G3,
>
> .tbls = {
> .serdes = msm8996_ufsphy_serdes,
> @@ -864,6 +912,7 @@ static const struct qmp_phy_cfg sa8775p_ufsphy_cfg = {
> .lanes = 2,
>
> .offsets = &qmp_ufs_offsets,
> + .max_supported_gear = UFS_HS_G4,
>
> .tbls = {
> .serdes = sm8350_ufsphy_serdes,
> @@ -879,13 +928,14 @@ static const struct qmp_phy_cfg sa8775p_ufsphy_cfg = {
> .serdes = sm8350_ufsphy_hs_b_serdes,
> .serdes_num = ARRAY_SIZE(sm8350_ufsphy_hs_b_serdes),
> },
> - .tbls_hs_g4 = {
> + .tbls_hs_overlay[0] = {
> .tx = sm8350_ufsphy_g4_tx,
> .tx_num = ARRAY_SIZE(sm8350_ufsphy_g4_tx),
> .rx = sm8350_ufsphy_g4_rx,
> .rx_num = ARRAY_SIZE(sm8350_ufsphy_g4_rx),
> .pcs = sm8350_ufsphy_g4_pcs,
> .pcs_num = ARRAY_SIZE(sm8350_ufsphy_g4_pcs),
> + .max_gear = UFS_HS_G4,
> },
> .clk_list = sm8450_ufs_phy_clk_l,
> .num_clks = ARRAY_SIZE(sm8450_ufs_phy_clk_l),
> @@ -898,6 +948,7 @@ static const struct qmp_phy_cfg sc8280xp_ufsphy_cfg = {
> .lanes = 2,
>
> .offsets = &qmp_ufs_offsets,
> + .max_supported_gear = UFS_HS_G4,
>
> .tbls = {
> .serdes = sm8350_ufsphy_serdes,
> @@ -913,13 +964,14 @@ static const struct qmp_phy_cfg sc8280xp_ufsphy_cfg = {
> .serdes = sm8350_ufsphy_hs_b_serdes,
> .serdes_num = ARRAY_SIZE(sm8350_ufsphy_hs_b_serdes),
> },
> - .tbls_hs_g4 = {
> + .tbls_hs_overlay[0] = {
> .tx = sm8350_ufsphy_g4_tx,
> .tx_num = ARRAY_SIZE(sm8350_ufsphy_g4_tx),
> .rx = sm8350_ufsphy_g4_rx,
> .rx_num = ARRAY_SIZE(sm8350_ufsphy_g4_rx),
> .pcs = sm8350_ufsphy_g4_pcs,
> .pcs_num = ARRAY_SIZE(sm8350_ufsphy_g4_pcs),
> + .max_gear = UFS_HS_G4,
> },
> .clk_list = sdm845_ufs_phy_clk_l,
> .num_clks = ARRAY_SIZE(sdm845_ufs_phy_clk_l),
> @@ -932,6 +984,7 @@ static const struct qmp_phy_cfg sdm845_ufsphy_cfg = {
> .lanes = 2,
>
> .offsets = &qmp_ufs_offsets,
> + .max_supported_gear = UFS_HS_G3,
>
> .tbls = {
> .serdes = sdm845_ufsphy_serdes,
> @@ -960,6 +1013,7 @@ static const struct qmp_phy_cfg sm6115_ufsphy_cfg = {
> .lanes = 1,
>
> .offsets = &qmp_ufs_offsets,
> + .max_supported_gear = UFS_HS_G3,
>
> .tbls = {
> .serdes = sm6115_ufsphy_serdes,
> @@ -988,6 +1042,7 @@ static const struct qmp_phy_cfg sm7150_ufsphy_cfg = {
> .lanes = 1,
>
> .offsets = &qmp_ufs_offsets,
> + .max_supported_gear = UFS_HS_G3,
>
> .tbls = {
> .serdes = sdm845_ufsphy_serdes,
> @@ -1016,6 +1071,7 @@ static const struct qmp_phy_cfg sm8150_ufsphy_cfg = {
> .lanes = 2,
>
> .offsets = &qmp_ufs_offsets,
> + .max_supported_gear = UFS_HS_G4,
>
> .tbls = {
> .serdes = sm8150_ufsphy_serdes,
> @@ -1031,13 +1087,14 @@ static const struct qmp_phy_cfg sm8150_ufsphy_cfg = {
> .serdes = sm8150_ufsphy_hs_b_serdes,
> .serdes_num = ARRAY_SIZE(sm8150_ufsphy_hs_b_serdes),
> },
> - .tbls_hs_g4 = {
> + .tbls_hs_overlay[0] = {
> .tx = sm8150_ufsphy_hs_g4_tx,
> .tx_num = ARRAY_SIZE(sm8150_ufsphy_hs_g4_tx),
> .rx = sm8150_ufsphy_hs_g4_rx,
> .rx_num = ARRAY_SIZE(sm8150_ufsphy_hs_g4_rx),
> .pcs = sm8150_ufsphy_hs_g4_pcs,
> .pcs_num = ARRAY_SIZE(sm8150_ufsphy_hs_g4_pcs),
> + .max_gear = UFS_HS_G4,
> },
> .clk_list = sdm845_ufs_phy_clk_l,
> .num_clks = ARRAY_SIZE(sdm845_ufs_phy_clk_l),
> @@ -1050,6 +1107,7 @@ static const struct qmp_phy_cfg sm8250_ufsphy_cfg = {
> .lanes = 2,
>
> .offsets = &qmp_ufs_offsets,
> + .max_supported_gear = UFS_HS_G4,
>
> .tbls = {
> .serdes = sm8150_ufsphy_serdes,
> @@ -1065,13 +1123,14 @@ static const struct qmp_phy_cfg sm8250_ufsphy_cfg = {
> .serdes = sm8150_ufsphy_hs_b_serdes,
> .serdes_num = ARRAY_SIZE(sm8150_ufsphy_hs_b_serdes),
> },
> - .tbls_hs_g4 = {
> + .tbls_hs_overlay[0] = {
> .tx = sm8250_ufsphy_hs_g4_tx,
> .tx_num = ARRAY_SIZE(sm8250_ufsphy_hs_g4_tx),
> .rx = sm8250_ufsphy_hs_g4_rx,
> .rx_num = ARRAY_SIZE(sm8250_ufsphy_hs_g4_rx),
> .pcs = sm8150_ufsphy_hs_g4_pcs,
> .pcs_num = ARRAY_SIZE(sm8150_ufsphy_hs_g4_pcs),
> + .max_gear = UFS_HS_G4,
> },
> .clk_list = sdm845_ufs_phy_clk_l,
> .num_clks = ARRAY_SIZE(sdm845_ufs_phy_clk_l),
> @@ -1084,6 +1143,7 @@ static const struct qmp_phy_cfg sm8350_ufsphy_cfg = {
> .lanes = 2,
>
> .offsets = &qmp_ufs_offsets,
> + .max_supported_gear = UFS_HS_G4,
>
> .tbls = {
> .serdes = sm8350_ufsphy_serdes,
> @@ -1099,13 +1159,14 @@ static const struct qmp_phy_cfg sm8350_ufsphy_cfg = {
> .serdes = sm8350_ufsphy_hs_b_serdes,
> .serdes_num = ARRAY_SIZE(sm8350_ufsphy_hs_b_serdes),
> },
> - .tbls_hs_g4 = {
> + .tbls_hs_overlay[0] = {
> .tx = sm8350_ufsphy_g4_tx,
> .tx_num = ARRAY_SIZE(sm8350_ufsphy_g4_tx),
> .rx = sm8350_ufsphy_g4_rx,
> .rx_num = ARRAY_SIZE(sm8350_ufsphy_g4_rx),
> .pcs = sm8350_ufsphy_g4_pcs,
> .pcs_num = ARRAY_SIZE(sm8350_ufsphy_g4_pcs),
> + .max_gear = UFS_HS_G4,
> },
> .clk_list = sdm845_ufs_phy_clk_l,
> .num_clks = ARRAY_SIZE(sdm845_ufs_phy_clk_l),
> @@ -1118,6 +1179,7 @@ static const struct qmp_phy_cfg sm8450_ufsphy_cfg = {
> .lanes = 2,
>
> .offsets = &qmp_ufs_offsets,
> + .max_supported_gear = UFS_HS_G4,
>
> .tbls = {
> .serdes = sm8350_ufsphy_serdes,
> @@ -1133,13 +1195,14 @@ static const struct qmp_phy_cfg sm8450_ufsphy_cfg = {
> .serdes = sm8350_ufsphy_hs_b_serdes,
> .serdes_num = ARRAY_SIZE(sm8350_ufsphy_hs_b_serdes),
> },
> - .tbls_hs_g4 = {
> + .tbls_hs_overlay[0] = {
> .tx = sm8350_ufsphy_g4_tx,
> .tx_num = ARRAY_SIZE(sm8350_ufsphy_g4_tx),
> .rx = sm8350_ufsphy_g4_rx,
> .rx_num = ARRAY_SIZE(sm8350_ufsphy_g4_rx),
> .pcs = sm8350_ufsphy_g4_pcs,
> .pcs_num = ARRAY_SIZE(sm8350_ufsphy_g4_pcs),
> + .max_gear = UFS_HS_G4,
> },
> .clk_list = sm8450_ufs_phy_clk_l,
> .num_clks = ARRAY_SIZE(sm8450_ufs_phy_clk_l),
> @@ -1152,6 +1215,7 @@ static const struct qmp_phy_cfg sm8550_ufsphy_cfg = {
> .lanes = 2,
>
> .offsets = &qmp_ufs_offsets_v6,
> + .max_supported_gear = UFS_HS_G5,
>
> .tbls = {
> .serdes = sm8550_ufsphy_serdes,
> @@ -1167,6 +1231,26 @@ static const struct qmp_phy_cfg sm8550_ufsphy_cfg = {
> .serdes = sm8550_ufsphy_hs_b_serdes,
> .serdes_num = ARRAY_SIZE(sm8550_ufsphy_hs_b_serdes),
> },
> + .tbls_hs_overlay[0] = {
> + .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),
> + .max_gear = UFS_HS_G4,
> + },
> + .tbls_hs_overlay[1] = {
> + .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),
> + .max_gear = UFS_HS_G5,
> + },
> .clk_list = sdm845_ufs_phy_clk_l,
> .num_clks = ARRAY_SIZE(sdm845_ufs_phy_clk_l),
> .vreg_list = qmp_phy_vreg_l,
> @@ -1229,17 +1313,63 @@ static void qmp_ufs_pcs_init(struct qmp_ufs *qmp, const struct qmp_phy_cfg_tbls
> qmp_ufs_configure(pcs, tbls->pcs, tbls->pcs_num);
> }
>
> -static void qmp_ufs_init_registers(struct qmp_ufs *qmp, const struct qmp_phy_cfg *cfg)
> +static bool qmp_ufs_match_gear_overlay(struct qmp_ufs *qmp, const struct qmp_phy_cfg *cfg, int *i)
> +{
> + u32 max_gear, floor_max_gear = cfg->max_supported_gear;
> + bool found = false;
> + int j;
> +
> + for (j = 0; j < NUM_OVERLAY; j ++) {
> + max_gear = cfg->tbls_hs_overlay[j].max_gear;
> +
> + if (max_gear == 0)
> + continue;
> +
> + /* Direct matching, bail */
> + if (qmp->submode == max_gear) {
> + *i = j;
> + return true;
> + }
> +
> + /* If no direct matching, the lowest gear is the best matching */
> + if (max_gear < floor_max_gear) {
> + *i = j;
> + found = true;
> + floor_max_gear = max_gear;
> + }
> + }
> +
> + return found;
> +}
> +
> +static int qmp_ufs_init_registers(struct qmp_ufs *qmp, const struct qmp_phy_cfg *cfg)
> {
> + bool apply_overlay;
> + int i;
> +
> + if (qmp->submode > cfg->max_supported_gear || qmp->submode == 0) {
> + dev_err(qmp->dev, "Invalid PHY submode %u\n", qmp->submode);
> + return -EINVAL;
> + }
> +
> + apply_overlay = qmp_ufs_match_gear_overlay(qmp, cfg, &i);
> +
> qmp_ufs_serdes_init(qmp, &cfg->tbls);
> + if (apply_overlay)
> + qmp_ufs_serdes_init(qmp, &cfg->tbls_hs_overlay[i]);
> +
> if (qmp->mode == PHY_MODE_UFS_HS_B)
> qmp_ufs_serdes_init(qmp, &cfg->tbls_hs_b);
> +
> qmp_ufs_lanes_init(qmp, &cfg->tbls);
> - if (qmp->submode == UFS_HS_G4)
> - qmp_ufs_lanes_init(qmp, &cfg->tbls_hs_g4);
> + if (apply_overlay)
> + qmp_ufs_lanes_init(qmp, &cfg->tbls_hs_overlay[i]);
> +
> qmp_ufs_pcs_init(qmp, &cfg->tbls);
> - if (qmp->submode == UFS_HS_G4)
> - qmp_ufs_pcs_init(qmp, &cfg->tbls_hs_g4);
> + if (apply_overlay)
> + qmp_ufs_pcs_init(qmp, &cfg->tbls_hs_overlay[i]);
> +
> + return 0;
> }
>
> static int qmp_ufs_com_init(struct qmp_ufs *qmp)
> @@ -1331,7 +1461,9 @@ static int qmp_ufs_power_on(struct phy *phy)
> unsigned int val;
> int ret;
>
> - qmp_ufs_init_registers(qmp, cfg);
> + ret = qmp_ufs_init_registers(qmp, cfg);
> + if (ret)
> + return ret;
>
> ret = reset_control_deassert(qmp->ufs_reset);
> if (ret)
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v5 09/10] phy: qualcomm: phy-qcom-qmp-ufs: Add High Speed Gear 5 support for SM8550
2023-11-28 9:59 ` neil.armstrong
@ 2023-11-28 10:03 ` Can Guo
0 siblings, 0 replies; 11+ messages in thread
From: Can Guo @ 2023-11-28 10:03 UTC (permalink / raw)
To: neil.armstrong, bvanassche, mani, adrian.hunter, beanhuo,
avri.altman, junwoo80.lee, martin.petersen
Cc: linux-scsi, linux-arm-msm, Andy Gross, Bjorn Andersson,
Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I,
open list:GENERIC PHY FRAMEWORK, open list
Hi Neil,
On 11/28/2023 5:59 PM, neil.armstrong@linaro.org wrote:
> Hi,
>
> On 23/11/2023 09:46, Can Guo wrote:
>> 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.
>>
>> To add HS-G5 support for SM8550, split the two sets of PHY settings into
>> their dedicated overlay tables, only the common parts of the two sets of
>> PHY settings are left in the .tbls.
>>
>> Consider we are going to add even higher gear support in future, to avoid
>> adding more tables with different names, rename the .tbls_hs_g4 and
>> make it
>> an array, a size of 2 is enough as of now.
>>
>> In this case, .tbls alone is not a complete set of PHY settings, so
>> either
>> tbls_hs_overlay[0] or tbls_hs_overlay[1] must be applied on top of the
>> .tbls to become a complete set of PHY settings.
>>
>> Signed-off-by: Can Guo <quic_cang@quicinc.com>
>
> Since Vinod has already merged SM8650 UFS PHY support, I'm afraid this
> serie will break UFS if the SM8650 UFS tables aren't updated aswell.
>
> Could you confirm if this will be the case ?
Because this change modifies data struct, it would be caught during
compiliation stage even if this change applies cleanly, hence it won't
break UFS.
Thanks,
Can Guo.
>
> Thanks,
> Neil
>
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 11+ messages in thread