[RFC PATCH v2 1/3] dt-bindings: phy: qcom,qmp-usb3-dp: Add sm6350 compatible

[RFC PATCH v2 1/3] dt-bindings: phy: qcom,qmp-usb3-dp: Add sm6350 compatible

[Luca Weiss]

Add the compatible describing the combo phy found on SM6350.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
@Johan Hovold, I've sent this v2 as RFC because there are several things
where I have questions on how it should be done.

In this patch, you can see there's cfg_ahb (&xo_board) and power-domains
is not set. In msm-4.19 &gcc_usb30_prim_gdsc is only used in the
ssusb@a600000 node, or should I also add it to qmpphy?

 .../bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml          | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
index 6f31693d9868..3e39e3e0504d 100644
--- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
@@ -17,16 +17,18 @@ properties:
   compatible:
     enum:
       - qcom,sc8280xp-qmp-usb43dp-phy
+      - qcom,sm6350-qmp-usb3-dp-phy
 
   reg:
     maxItems: 1
 
   clocks:
-    maxItems: 4
+    maxItems: 5
 
   clock-names:
     items:
       - const: aux
+      - const: cfg_ahb
       - const: ref
       - const: com_aux
       - const: usb3_pipe
@@ -61,7 +63,6 @@ required:
   - reg
   - clocks
   - clock-names
-  - power-domains
   - resets
   - reset-names
   - vdda-phy-supply
-- 
2.38.1

[RFC PATCH v2 1/3] dt-bindings: phy: qcom,qmp-usb3-dp: Add sm6350 compatible

[Luca Weiss]

Add the compatible describing the combo phy found on SM6350.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
@Johan Hovold, I've sent this v2 as RFC because there are several things
where I have questions on how it should be done.

In this patch, you can see there's cfg_ahb (&xo_board) and power-domains
is not set. In msm-4.19 &gcc_usb30_prim_gdsc is only used in the
ssusb@a600000 node, or should I also add it to qmpphy?

 .../bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml          | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
index 6f31693d9868..3e39e3e0504d 100644
--- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
@@ -17,16 +17,18 @@ properties:
   compatible:
     enum:
       - qcom,sc8280xp-qmp-usb43dp-phy
+      - qcom,sm6350-qmp-usb3-dp-phy
 
   reg:
     maxItems: 1
 
   clocks:
-    maxItems: 4
+    maxItems: 5
 
   clock-names:
     items:
       - const: aux
+      - const: cfg_ahb
       - const: ref
       - const: com_aux
       - const: usb3_pipe
@@ -61,7 +63,6 @@ required:
   - reg
   - clocks
   - clock-names
-  - power-domains
   - resets
   - reset-names
   - vdda-phy-supply
-- 
2.38.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[RFC PATCH v2 2/3] phy: qcom-qmp-combo: Add config for SM6350

[Luca Weiss]

Add the tables and config for the combo phy found on SM6350.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
@Johan Hovold, here I've added dp_txa & dp_txb, I believe otherwise
qmp->dp_tx would be wrong. Is this different on sc8280xp or was this a
mistake on your side? I think this should probably be split out to
another patch to not mix things up too much.

I think other than that this patch is good.

 drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 134 +++++++++++++++++++++-
 1 file changed, 132 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index 77052c66cf70..58c241b8844a 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -311,6 +311,70 @@ static const struct qmp_phy_init_tbl qmp_v3_usb3_pcs_tbl[] = {
 	QMP_PHY_INIT_CFG(QPHY_V3_PCS_RXEQTRAINING_RUN_TIME, 0x13),
 };
 
+static const struct qmp_phy_init_tbl sm6350_usb3_rx_tbl[] = {
+	QMP_PHY_INIT_CFG(QSERDES_V3_RX_UCDR_FASTLOCK_FO_GAIN, 0x0b),
+	QMP_PHY_INIT_CFG(QSERDES_V3_RX_RX_EQU_ADAPTOR_CNTRL2, 0x0f),
+	QMP_PHY_INIT_CFG(QSERDES_V3_RX_RX_EQU_ADAPTOR_CNTRL3, 0x4e),
+	QMP_PHY_INIT_CFG(QSERDES_V3_RX_RX_EQU_ADAPTOR_CNTRL4, 0x18),
+	QMP_PHY_INIT_CFG(QSERDES_V3_RX_RX_EQ_OFFSET_ADAPTOR_CNTRL1, 0x77),
+	QMP_PHY_INIT_CFG(QSERDES_V3_RX_RX_OFFSET_ADAPTOR_CNTRL2, 0x80),
+	QMP_PHY_INIT_CFG(QSERDES_V3_RX_SIGDET_CNTRL, 0x03),
+	QMP_PHY_INIT_CFG(QSERDES_V3_RX_SIGDET_DEGLITCH_CNTRL, 0x16),
+	QMP_PHY_INIT_CFG(QSERDES_V3_RX_RX_MODE_00, 0x05),
+	QMP_PHY_INIT_CFG(QSERDES_V3_RX_UCDR_SO_SATURATION_AND_ENABLE, 0x75),
+};
+
+static const struct qmp_phy_init_tbl sm6350_usb3_pcs_tbl[] = {
+	/* FLL settings */
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_FLL_CNTRL2, 0x83),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_FLL_CNT_VAL_L, 0x09),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_FLL_CNT_VAL_H_TOL, 0xa2),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_FLL_MAN_CODE, 0x40),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_FLL_CNTRL1, 0x02),
+
+	/* Lock Det settings */
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_LOCK_DETECT_CONFIG1, 0xd1),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_LOCK_DETECT_CONFIG2, 0x1f),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_LOCK_DETECT_CONFIG3, 0x47),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_POWER_STATE_CONFIG2, 0x1b),
+
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_RX_SIGDET_LVL, 0xcc),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXMGN_V0, 0x9f),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXMGN_V1, 0x9f),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXMGN_V2, 0xb7),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXMGN_V3, 0x4e),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXMGN_V4, 0x65),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXMGN_LS, 0x6b),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXDEEMPH_M6DB_V0, 0x15),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXDEEMPH_M3P5DB_V0, 0x0d),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXDEEMPH_M6DB_V1, 0x15),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXDEEMPH_M3P5DB_V1, 0x0d),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXDEEMPH_M6DB_V2, 0x15),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXDEEMPH_M3P5DB_V2, 0x0d),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXDEEMPH_M6DB_V3, 0x15),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXDEEMPH_M3P5DB_V3, 0x1d),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXDEEMPH_M6DB_V4, 0x15),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXDEEMPH_M3P5DB_V4, 0x0d),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXDEEMPH_M6DB_LS, 0x15),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXDEEMPH_M3P5DB_LS, 0x0d),
+
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_RATE_SLEW_CNTRL, 0x02),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_PWRUP_RESET_DLY_TIME_AUXCLK, 0x04),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TSYNC_RSYNC_TIME, 0x44),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_PWRUP_RESET_DLY_TIME_AUXCLK, 0x04),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_RCVR_DTCT_DLY_P1U2_L, 0xe7),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_RCVR_DTCT_DLY_P1U2_H, 0x03),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_RCVR_DTCT_DLY_U3_L, 0x40),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_RCVR_DTCT_DLY_U3_H, 0x00),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_RXEQTRAINING_WAIT_TIME, 0x75),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_LFPS_TX_ECSTART_EQTLOCK, 0x86),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_RXEQTRAINING_RUN_TIME, 0x13),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_LFPS_DET_HIGH_COUNT_VAL, 0x04),
+
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_REFGEN_REQ_CONFIG1, 0x21),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_REFGEN_REQ_CONFIG2, 0x60),
+};
+
 static const struct qmp_phy_init_tbl sm8150_usb3_serdes_tbl[] = {
 	QMP_PHY_INIT_CFG(QSERDES_V4_COM_SSC_EN_CENTER, 0x01),
 	QMP_PHY_INIT_CFG(QSERDES_V4_COM_SSC_PER1, 0x31),
@@ -809,6 +873,8 @@ struct qmp_combo_offsets {
 	u16 usb3_pcs;
 	u16 usb3_pcs_usb;
 	u16 dp_serdes;
+	u16 dp_txa;
+	u16 dp_txb;
 	u16 dp_dp_phy;
 };
 
@@ -975,6 +1041,21 @@ static const char * const sc7180_usb3phy_reset_l[] = {
 	"phy",
 };
 
+static const struct qmp_combo_offsets qmp_combo_offsets_v3 = {
+	.com		= 0x0000,
+	.txa		= 0x1200,
+	.rxa		= 0x1400,
+	.txb		= 0x1600,
+	.rxb		= 0x1800,
+	.usb3_serdes	= 0x1000,
+	.usb3_pcs_misc	= 0x1a00,
+	.usb3_pcs	= 0x1c00,
+	.dp_serdes	= 0x2000,
+	.dp_txa		= 0x2200,
+	.dp_txb		= 0x2600,
+	.dp_dp_phy	= 0x2c00,
+};
+
 static const struct qmp_combo_offsets qmp_combo_offsets_v5 = {
 	.com		= 0x0000,
 	.txa		= 0x0400,
@@ -1172,6 +1253,51 @@ static const struct qmp_phy_cfg sc8280xp_usb43dpphy_cfg = {
 	.regs			= qmp_v4_usb3phy_regs_layout,
 };
 
+static const struct qmp_phy_cfg sm6350_usb3dpphy_cfg = {
+	.offsets		= &qmp_combo_offsets_v3,
+
+	.serdes_tbl		= qmp_v3_usb3_serdes_tbl,
+	.serdes_tbl_num		= ARRAY_SIZE(qmp_v3_usb3_serdes_tbl),
+	.tx_tbl			= qmp_v3_usb3_tx_tbl,
+	.tx_tbl_num		= ARRAY_SIZE(qmp_v3_usb3_tx_tbl),
+	.rx_tbl			= sm6350_usb3_rx_tbl,
+	.rx_tbl_num		= ARRAY_SIZE(sm6350_usb3_rx_tbl),
+	.pcs_tbl		= sm6350_usb3_pcs_tbl,
+	.pcs_tbl_num		= ARRAY_SIZE(sm6350_usb3_pcs_tbl),
+
+	.dp_serdes_tbl		= qmp_v3_dp_serdes_tbl,
+	.dp_serdes_tbl_num	= ARRAY_SIZE(qmp_v3_dp_serdes_tbl),
+	.dp_tx_tbl		= qmp_v3_dp_tx_tbl,
+	.dp_tx_tbl_num		= ARRAY_SIZE(qmp_v3_dp_tx_tbl),
+
+	.serdes_tbl_rbr		= qmp_v3_dp_serdes_tbl_rbr,
+	.serdes_tbl_rbr_num	= ARRAY_SIZE(qmp_v3_dp_serdes_tbl_rbr),
+	.serdes_tbl_hbr		= qmp_v3_dp_serdes_tbl_hbr,
+	.serdes_tbl_hbr_num	= ARRAY_SIZE(qmp_v3_dp_serdes_tbl_hbr),
+	.serdes_tbl_hbr2	= qmp_v3_dp_serdes_tbl_hbr2,
+	.serdes_tbl_hbr2_num	= ARRAY_SIZE(qmp_v3_dp_serdes_tbl_hbr2),
+	.serdes_tbl_hbr3	= qmp_v3_dp_serdes_tbl_hbr3,
+	.serdes_tbl_hbr3_num	= ARRAY_SIZE(qmp_v3_dp_serdes_tbl_hbr3),
+
+	.swing_hbr_rbr		= &qmp_dp_v3_voltage_swing_hbr_rbr,
+	.pre_emphasis_hbr_rbr	= &qmp_dp_v3_pre_emphasis_hbr_rbr,
+	.swing_hbr3_hbr2	= &qmp_dp_v3_voltage_swing_hbr3_hbr2,
+	.pre_emphasis_hbr3_hbr2 = &qmp_dp_v3_pre_emphasis_hbr3_hbr2,
+
+	.dp_aux_init		= qmp_v3_dp_aux_init,
+	.configure_dp_tx	= qmp_v3_configure_dp_tx,
+	.configure_dp_phy	= qmp_v3_configure_dp_phy,
+	.calibrate_dp_phy	= qmp_v3_calibrate_dp_phy,
+
+	.clk_list		= qmp_v3_phy_clk_l,
+	.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		= qmp_phy_vreg_l,
+	.num_vregs		= ARRAY_SIZE(qmp_phy_vreg_l),
+	.regs			= qmp_v3_usb3phy_regs_layout,
+};
+
 static const struct qmp_phy_cfg sm8250_usb3dpphy_cfg = {
 	.serdes_tbl		= sm8150_usb3_serdes_tbl,
 	.serdes_tbl_num		= ARRAY_SIZE(sm8150_usb3_serdes_tbl),
@@ -2641,8 +2767,8 @@ static int qmp_combo_parse_dt(struct qmp_combo *qmp)
 	qmp->pcs_usb = base + offs->usb3_pcs_usb;
 
 	qmp->dp_serdes = base + offs->dp_serdes;
-	qmp->dp_tx = base + offs->txa;
-	qmp->dp_tx2 = base + offs->txb;
+	qmp->dp_tx = base + offs->dp_txa;
+	qmp->dp_tx2 = base + offs->dp_txb;
 	qmp->dp_dp_phy = base + offs->dp_dp_phy;
 
 	qmp->pipe_clk = devm_clk_get(dev, "usb3_pipe");
@@ -2789,6 +2915,10 @@ static const struct of_device_id qmp_combo_of_match_table[] = {
 		.compatible = "qcom,sdm845-qmp-usb3-dp-phy",
 		.data = &sdm845_usb3dpphy_cfg,
 	},
+	{
+		.compatible = "qcom,sm6350-qmp-usb3-dp-phy",
+		.data = &sm6350_usb3dpphy_cfg,
+	},
 	{
 		.compatible = "qcom,sm8250-qmp-usb3-dp-phy",
 		.data = &sm8250_usb3dpphy_cfg,
-- 
2.38.1

[RFC PATCH v2 2/3] phy: qcom-qmp-combo: Add config for SM6350

[Luca Weiss]

Add the tables and config for the combo phy found on SM6350.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
@Johan Hovold, here I've added dp_txa & dp_txb, I believe otherwise
qmp->dp_tx would be wrong. Is this different on sc8280xp or was this a
mistake on your side? I think this should probably be split out to
another patch to not mix things up too much.

I think other than that this patch is good.

 drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 134 +++++++++++++++++++++-
 1 file changed, 132 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index 77052c66cf70..58c241b8844a 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -311,6 +311,70 @@ static const struct qmp_phy_init_tbl qmp_v3_usb3_pcs_tbl[] = {
 	QMP_PHY_INIT_CFG(QPHY_V3_PCS_RXEQTRAINING_RUN_TIME, 0x13),
 };
 
+static const struct qmp_phy_init_tbl sm6350_usb3_rx_tbl[] = {
+	QMP_PHY_INIT_CFG(QSERDES_V3_RX_UCDR_FASTLOCK_FO_GAIN, 0x0b),
+	QMP_PHY_INIT_CFG(QSERDES_V3_RX_RX_EQU_ADAPTOR_CNTRL2, 0x0f),
+	QMP_PHY_INIT_CFG(QSERDES_V3_RX_RX_EQU_ADAPTOR_CNTRL3, 0x4e),
+	QMP_PHY_INIT_CFG(QSERDES_V3_RX_RX_EQU_ADAPTOR_CNTRL4, 0x18),
+	QMP_PHY_INIT_CFG(QSERDES_V3_RX_RX_EQ_OFFSET_ADAPTOR_CNTRL1, 0x77),
+	QMP_PHY_INIT_CFG(QSERDES_V3_RX_RX_OFFSET_ADAPTOR_CNTRL2, 0x80),
+	QMP_PHY_INIT_CFG(QSERDES_V3_RX_SIGDET_CNTRL, 0x03),
+	QMP_PHY_INIT_CFG(QSERDES_V3_RX_SIGDET_DEGLITCH_CNTRL, 0x16),
+	QMP_PHY_INIT_CFG(QSERDES_V3_RX_RX_MODE_00, 0x05),
+	QMP_PHY_INIT_CFG(QSERDES_V3_RX_UCDR_SO_SATURATION_AND_ENABLE, 0x75),
+};
+
+static const struct qmp_phy_init_tbl sm6350_usb3_pcs_tbl[] = {
+	/* FLL settings */
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_FLL_CNTRL2, 0x83),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_FLL_CNT_VAL_L, 0x09),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_FLL_CNT_VAL_H_TOL, 0xa2),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_FLL_MAN_CODE, 0x40),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_FLL_CNTRL1, 0x02),
+
+	/* Lock Det settings */
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_LOCK_DETECT_CONFIG1, 0xd1),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_LOCK_DETECT_CONFIG2, 0x1f),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_LOCK_DETECT_CONFIG3, 0x47),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_POWER_STATE_CONFIG2, 0x1b),
+
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_RX_SIGDET_LVL, 0xcc),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXMGN_V0, 0x9f),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXMGN_V1, 0x9f),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXMGN_V2, 0xb7),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXMGN_V3, 0x4e),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXMGN_V4, 0x65),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXMGN_LS, 0x6b),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXDEEMPH_M6DB_V0, 0x15),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXDEEMPH_M3P5DB_V0, 0x0d),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXDEEMPH_M6DB_V1, 0x15),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXDEEMPH_M3P5DB_V1, 0x0d),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXDEEMPH_M6DB_V2, 0x15),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXDEEMPH_M3P5DB_V2, 0x0d),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXDEEMPH_M6DB_V3, 0x15),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXDEEMPH_M3P5DB_V3, 0x1d),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXDEEMPH_M6DB_V4, 0x15),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXDEEMPH_M3P5DB_V4, 0x0d),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXDEEMPH_M6DB_LS, 0x15),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TXDEEMPH_M3P5DB_LS, 0x0d),
+
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_RATE_SLEW_CNTRL, 0x02),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_PWRUP_RESET_DLY_TIME_AUXCLK, 0x04),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_TSYNC_RSYNC_TIME, 0x44),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_PWRUP_RESET_DLY_TIME_AUXCLK, 0x04),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_RCVR_DTCT_DLY_P1U2_L, 0xe7),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_RCVR_DTCT_DLY_P1U2_H, 0x03),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_RCVR_DTCT_DLY_U3_L, 0x40),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_RCVR_DTCT_DLY_U3_H, 0x00),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_RXEQTRAINING_WAIT_TIME, 0x75),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_LFPS_TX_ECSTART_EQTLOCK, 0x86),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_RXEQTRAINING_RUN_TIME, 0x13),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_LFPS_DET_HIGH_COUNT_VAL, 0x04),
+
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_REFGEN_REQ_CONFIG1, 0x21),
+	QMP_PHY_INIT_CFG(QPHY_V3_PCS_REFGEN_REQ_CONFIG2, 0x60),
+};
+
 static const struct qmp_phy_init_tbl sm8150_usb3_serdes_tbl[] = {
 	QMP_PHY_INIT_CFG(QSERDES_V4_COM_SSC_EN_CENTER, 0x01),
 	QMP_PHY_INIT_CFG(QSERDES_V4_COM_SSC_PER1, 0x31),
@@ -809,6 +873,8 @@ struct qmp_combo_offsets {
 	u16 usb3_pcs;
 	u16 usb3_pcs_usb;
 	u16 dp_serdes;
+	u16 dp_txa;
+	u16 dp_txb;
 	u16 dp_dp_phy;
 };
 
@@ -975,6 +1041,21 @@ static const char * const sc7180_usb3phy_reset_l[] = {
 	"phy",
 };
 
+static const struct qmp_combo_offsets qmp_combo_offsets_v3 = {
+	.com		= 0x0000,
+	.txa		= 0x1200,
+	.rxa		= 0x1400,
+	.txb		= 0x1600,
+	.rxb		= 0x1800,
+	.usb3_serdes	= 0x1000,
+	.usb3_pcs_misc	= 0x1a00,
+	.usb3_pcs	= 0x1c00,
+	.dp_serdes	= 0x2000,
+	.dp_txa		= 0x2200,
+	.dp_txb		= 0x2600,
+	.dp_dp_phy	= 0x2c00,
+};
+
 static const struct qmp_combo_offsets qmp_combo_offsets_v5 = {
 	.com		= 0x0000,
 	.txa		= 0x0400,
@@ -1172,6 +1253,51 @@ static const struct qmp_phy_cfg sc8280xp_usb43dpphy_cfg = {
 	.regs			= qmp_v4_usb3phy_regs_layout,
 };
 
+static const struct qmp_phy_cfg sm6350_usb3dpphy_cfg = {
+	.offsets		= &qmp_combo_offsets_v3,
+
+	.serdes_tbl		= qmp_v3_usb3_serdes_tbl,
+	.serdes_tbl_num		= ARRAY_SIZE(qmp_v3_usb3_serdes_tbl),
+	.tx_tbl			= qmp_v3_usb3_tx_tbl,
+	.tx_tbl_num		= ARRAY_SIZE(qmp_v3_usb3_tx_tbl),
+	.rx_tbl			= sm6350_usb3_rx_tbl,
+	.rx_tbl_num		= ARRAY_SIZE(sm6350_usb3_rx_tbl),
+	.pcs_tbl		= sm6350_usb3_pcs_tbl,
+	.pcs_tbl_num		= ARRAY_SIZE(sm6350_usb3_pcs_tbl),
+
+	.dp_serdes_tbl		= qmp_v3_dp_serdes_tbl,
+	.dp_serdes_tbl_num	= ARRAY_SIZE(qmp_v3_dp_serdes_tbl),
+	.dp_tx_tbl		= qmp_v3_dp_tx_tbl,
+	.dp_tx_tbl_num		= ARRAY_SIZE(qmp_v3_dp_tx_tbl),
+
+	.serdes_tbl_rbr		= qmp_v3_dp_serdes_tbl_rbr,
+	.serdes_tbl_rbr_num	= ARRAY_SIZE(qmp_v3_dp_serdes_tbl_rbr),
+	.serdes_tbl_hbr		= qmp_v3_dp_serdes_tbl_hbr,
+	.serdes_tbl_hbr_num	= ARRAY_SIZE(qmp_v3_dp_serdes_tbl_hbr),
+	.serdes_tbl_hbr2	= qmp_v3_dp_serdes_tbl_hbr2,
+	.serdes_tbl_hbr2_num	= ARRAY_SIZE(qmp_v3_dp_serdes_tbl_hbr2),
+	.serdes_tbl_hbr3	= qmp_v3_dp_serdes_tbl_hbr3,
+	.serdes_tbl_hbr3_num	= ARRAY_SIZE(qmp_v3_dp_serdes_tbl_hbr3),
+
+	.swing_hbr_rbr		= &qmp_dp_v3_voltage_swing_hbr_rbr,
+	.pre_emphasis_hbr_rbr	= &qmp_dp_v3_pre_emphasis_hbr_rbr,
+	.swing_hbr3_hbr2	= &qmp_dp_v3_voltage_swing_hbr3_hbr2,
+	.pre_emphasis_hbr3_hbr2 = &qmp_dp_v3_pre_emphasis_hbr3_hbr2,
+
+	.dp_aux_init		= qmp_v3_dp_aux_init,
+	.configure_dp_tx	= qmp_v3_configure_dp_tx,
+	.configure_dp_phy	= qmp_v3_configure_dp_phy,
+	.calibrate_dp_phy	= qmp_v3_calibrate_dp_phy,
+
+	.clk_list		= qmp_v3_phy_clk_l,
+	.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		= qmp_phy_vreg_l,
+	.num_vregs		= ARRAY_SIZE(qmp_phy_vreg_l),
+	.regs			= qmp_v3_usb3phy_regs_layout,
+};
+
 static const struct qmp_phy_cfg sm8250_usb3dpphy_cfg = {
 	.serdes_tbl		= sm8150_usb3_serdes_tbl,
 	.serdes_tbl_num		= ARRAY_SIZE(sm8150_usb3_serdes_tbl),
@@ -2641,8 +2767,8 @@ static int qmp_combo_parse_dt(struct qmp_combo *qmp)
 	qmp->pcs_usb = base + offs->usb3_pcs_usb;
 
 	qmp->dp_serdes = base + offs->dp_serdes;
-	qmp->dp_tx = base + offs->txa;
-	qmp->dp_tx2 = base + offs->txb;
+	qmp->dp_tx = base + offs->dp_txa;
+	qmp->dp_tx2 = base + offs->dp_txb;
 	qmp->dp_dp_phy = base + offs->dp_dp_phy;
 
 	qmp->pipe_clk = devm_clk_get(dev, "usb3_pipe");
@@ -2789,6 +2915,10 @@ static const struct of_device_id qmp_combo_of_match_table[] = {
 		.compatible = "qcom,sdm845-qmp-usb3-dp-phy",
 		.data = &sdm845_usb3dpphy_cfg,
 	},
+	{
+		.compatible = "qcom,sm6350-qmp-usb3-dp-phy",
+		.data = &sm6350_usb3dpphy_cfg,
+	},
 	{
 		.compatible = "qcom,sm8250-qmp-usb3-dp-phy",
 		.data = &sm8250_usb3dpphy_cfg,
-- 
2.38.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[RFC PATCH v2 3/3] arm64: dts: qcom: sm6350: Use specific qmpphy compatible

[Luca Weiss]

The sc7180 phy compatible works fine for some cases, but it turns out
sm6350 does need proper phy configuration in the driver, so use the
newly added sm6350 compatible.

Because the sm6350 compatible is using the new binding, we need to
change the node quite a bit to match it.

This fixes qmpphy init when no USB cable is plugged in during bootloader
stage.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
@Johan Hovold, in this patch there's also the question about cfg_ahb,
power-domains but I'm also not happy about using the
QMP_USB43DP_USB3_PHY define for the phy reference. Do you think it's a
good idea to introduce e.g. QMP_USB3DP_USB3_PHY with the same value so
it's essentially just an alias to the other?

This series is tested on next-20221124 with next branch of linux-phy
repo (commit bea3ce759b46) merged in.

 arch/arm64/boot/dts/qcom/sm6350.dtsi | 46 +++++++---------------------
 1 file changed, 11 insertions(+), 35 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm6350.dtsi b/arch/arm64/boot/dts/qcom/sm6350.dtsi
index 0f01ff4feb55..923c8bb7e5f8 100644
--- a/arch/arm64/boot/dts/qcom/sm6350.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm6350.dtsi
@@ -11,6 +11,7 @@
 #include <dt-bindings/interconnect/qcom,sm6350.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/mailbox/qcom-ipcc.h>
+#include <dt-bindings/phy/phy-qcom-qmp.h>
 #include <dt-bindings/power/qcom-rpmpd.h>
 #include <dt-bindings/soc/qcom,rpmh-rsc.h>
 
@@ -1119,50 +1120,25 @@ usb_1_hsphy: phy@88e3000 {
 			resets = <&gcc GCC_QUSB2PHY_PRIM_BCR>;
 		};
 
-		usb_1_qmpphy: phy@88e9000 {
-			compatible = "qcom,sc7180-qmp-usb3-dp-phy";
-			reg = <0 0x088e9000 0 0x200>,
-			      <0 0x088e8000 0 0x40>,
-			      <0 0x088ea000 0 0x200>;
-			status = "disabled";
-			#address-cells = <2>;
-			#size-cells = <2>;
-			ranges;
+		usb_1_qmpphy: phy@88e8000 {
+			compatible = "qcom,sm6350-qmp-usb3-dp-phy";
+			reg = <0 0x088e8000 0 0x3000>;
 
 			clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
 				 <&xo_board>,
 				 <&rpmhcc RPMH_QLINK_CLK>,
-				 <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>;
-			clock-names = "aux", "cfg_ahb", "ref", "com_aux";
+				 <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
+				 <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
+			clock-names = "aux", "cfg_ahb", "ref", "com_aux", "usb3_pipe";
 
 			resets = <&gcc GCC_USB3_DP_PHY_PRIM_BCR>,
 				 <&gcc GCC_USB3_PHY_PRIM_BCR>;
 			reset-names = "phy", "common";
 
-			usb_1_ssphy: usb3-phy@88e9200 {
-				reg = <0 0x088e9200 0 0x200>,
-				      <0 0x088e9400 0 0x200>,
-				      <0 0x088e9c00 0 0x400>,
-				      <0 0x088e9600 0 0x200>,
-				      <0 0x088e9800 0 0x200>,
-				      <0 0x088e9a00 0 0x100>;
-				#clock-cells = <0>;
-				#phy-cells = <0>;
-				clocks = <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
-				clock-names = "pipe0";
-				clock-output-names = "usb3_phy_pipe_clk_src";
-			};
+			#clock-cells = <1>;
+			#phy-cells = <1>;
 
-			dp_phy: dp-phy@88ea200 {
-				reg = <0 0x088ea200 0 0x200>,
-				      <0 0x088ea400 0 0x200>,
-				      <0 0x088eac00 0 0x400>,
-				      <0 0x088ea600 0 0x200>,
-				      <0 0x088ea800 0 0x200>,
-				      <0 0x088eaa00 0 0x100>;
-				#phy-cells = <0>;
-				#clock-cells = <1>;
-			};
+			status = "disabled";
 		};
 
 		dc_noc: interconnect@9160000 {
@@ -1236,7 +1212,7 @@ usb_1_dwc3: usb@a600000 {
 				snps,dis_enblslpm_quirk;
 				snps,has-lpm-erratum;
 				snps,hird-threshold = /bits/ 8 <0x10>;
-				phys = <&usb_1_hsphy>, <&usb_1_ssphy>;
+				phys = <&usb_1_hsphy>, <&usb_1_qmpphy QMP_USB43DP_USB3_PHY>;
 				phy-names = "usb2-phy", "usb3-phy";
 			};
 		};
-- 
2.38.1

Re: [RFC PATCH v2 1/3] dt-bindings: phy: qcom,qmp-usb3-dp: Add sm6350 compatible

[Johan Hovold]

On Fri, Nov 25, 2022 at 10:27:47AM +0100, Luca Weiss wrote:
> Add the compatible describing the combo phy found on SM6350.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
> @Johan Hovold, I've sent this v2 as RFC because there are several things
> where I have questions on how it should be done.
> 
> In this patch, you can see there's cfg_ahb (&xo_board) and power-domains
> is not set. In msm-4.19 &gcc_usb30_prim_gdsc is only used in the
> ssusb@a600000 node, or should I also add it to qmpphy?

Yeah, you may need to add a platform specific section of the clocks,
which appear to be different, even if I'm not sure they are currently
described correctly (xo_board as cfg_ahb and "QLINK" as ref). How are
they named in the vendor's dts?

It should be OK to include the power-domain also for the PHY node.

>  .../bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml          | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> index 6f31693d9868..3e39e3e0504d 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> @@ -17,16 +17,18 @@ properties:
>    compatible:
>      enum:
>        - qcom,sc8280xp-qmp-usb43dp-phy
> +      - qcom,sm6350-qmp-usb3-dp-phy
>  
>    reg:
>      maxItems: 1
>  
>    clocks:
> -    maxItems: 4
> +    maxItems: 5
>  
>    clock-names:
>      items:
>        - const: aux
> +      - const: cfg_ahb
>        - const: ref
>        - const: com_aux
>        - const: usb3_pipe

So this would need to be moved to an allOf: construct at the end with
one section each for sc8280xp and sm6350.

> @@ -61,7 +63,6 @@ required:
>    - reg
>    - clocks
>    - clock-names
> -  - power-domains
>    - resets
>    - reset-names
>    - vdda-phy-supply

Johan

Re: [RFC PATCH v2 1/3] dt-bindings: phy: qcom,qmp-usb3-dp: Add sm6350 compatible

[Johan Hovold]

On Fri, Nov 25, 2022 at 10:27:47AM +0100, Luca Weiss wrote:
> Add the compatible describing the combo phy found on SM6350.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
> @Johan Hovold, I've sent this v2 as RFC because there are several things
> where I have questions on how it should be done.
> 
> In this patch, you can see there's cfg_ahb (&xo_board) and power-domains
> is not set. In msm-4.19 &gcc_usb30_prim_gdsc is only used in the
> ssusb@a600000 node, or should I also add it to qmpphy?

Yeah, you may need to add a platform specific section of the clocks,
which appear to be different, even if I'm not sure they are currently
described correctly (xo_board as cfg_ahb and "QLINK" as ref). How are
they named in the vendor's dts?

It should be OK to include the power-domain also for the PHY node.

>  .../bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml          | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> index 6f31693d9868..3e39e3e0504d 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> @@ -17,16 +17,18 @@ properties:
>    compatible:
>      enum:
>        - qcom,sc8280xp-qmp-usb43dp-phy
> +      - qcom,sm6350-qmp-usb3-dp-phy
>  
>    reg:
>      maxItems: 1
>  
>    clocks:
> -    maxItems: 4
> +    maxItems: 5
>  
>    clock-names:
>      items:
>        - const: aux
> +      - const: cfg_ahb
>        - const: ref
>        - const: com_aux
>        - const: usb3_pipe

So this would need to be moved to an allOf: construct at the end with
one section each for sc8280xp and sm6350.

> @@ -61,7 +63,6 @@ required:
>    - reg
>    - clocks
>    - clock-names
> -  - power-domains
>    - resets
>    - reset-names
>    - vdda-phy-supply

Johan

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RFC PATCH v2 1/3] dt-bindings: phy: qcom,qmp-usb3-dp: Add sm6350 compatible

[Luca Weiss]

Hi Johan,

On Fri Nov 25, 2022 at 10:50 AM CET, Johan Hovold wrote:
> On Fri, Nov 25, 2022 at 10:27:47AM +0100, Luca Weiss wrote:
> > Add the compatible describing the combo phy found on SM6350.
> > 
> > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > ---
> > @Johan Hovold, I've sent this v2 as RFC because there are several things
> > where I have questions on how it should be done.
> > 
> > In this patch, you can see there's cfg_ahb (&xo_board) and power-domains
> > is not set. In msm-4.19 &gcc_usb30_prim_gdsc is only used in the
> > ssusb@a600000 node, or should I also add it to qmpphy?
>
> Yeah, you may need to add a platform specific section of the clocks,
> which appear to be different, even if I'm not sure they are currently
> described correctly (xo_board as cfg_ahb and "QLINK" as ref). How are
> they named in the vendor's dts?

This is the msm-4.19 dts:
https://android.googlesource.com/kernel/msm-extra/devicetree/+/refs/heads/android-msm-bramble-4.19-android11-qpr1/qcom/lagoon-usb.dtsi#354

>
> It should be OK to include the power-domain also for the PHY node.

Ack.

>
> >  .../bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml          | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> > index 6f31693d9868..3e39e3e0504d 100644
> > --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> > +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> > @@ -17,16 +17,18 @@ properties:
> >    compatible:
> >      enum:
> >        - qcom,sc8280xp-qmp-usb43dp-phy
> > +      - qcom,sm6350-qmp-usb3-dp-phy
> >  
> >    reg:
> >      maxItems: 1
> >  
> >    clocks:
> > -    maxItems: 4
> > +    maxItems: 5
> >  
> >    clock-names:
> >      items:
> >        - const: aux
> > +      - const: cfg_ahb
> >        - const: ref
> >        - const: com_aux
> >        - const: usb3_pipe
>
> So this would need to be moved to an allOf: construct at the end with
> one section each for sc8280xp and sm6350.

Ack.

Thanks for the quick response!

Regards,
Luca

>
> > @@ -61,7 +63,6 @@ required:
> >    - reg
> >    - clocks
> >    - clock-names
> > -  - power-domains
> >    - resets
> >    - reset-names
> >    - vdda-phy-supply
>
> Johan

Re: [RFC PATCH v2 1/3] dt-bindings: phy: qcom,qmp-usb3-dp: Add sm6350 compatible

[Luca Weiss]

Hi Johan,

On Fri Nov 25, 2022 at 10:50 AM CET, Johan Hovold wrote:
> On Fri, Nov 25, 2022 at 10:27:47AM +0100, Luca Weiss wrote:
> > Add the compatible describing the combo phy found on SM6350.
> > 
> > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > ---
> > @Johan Hovold, I've sent this v2 as RFC because there are several things
> > where I have questions on how it should be done.
> > 
> > In this patch, you can see there's cfg_ahb (&xo_board) and power-domains
> > is not set. In msm-4.19 &gcc_usb30_prim_gdsc is only used in the
> > ssusb@a600000 node, or should I also add it to qmpphy?
>
> Yeah, you may need to add a platform specific section of the clocks,
> which appear to be different, even if I'm not sure they are currently
> described correctly (xo_board as cfg_ahb and "QLINK" as ref). How are
> they named in the vendor's dts?

This is the msm-4.19 dts:
https://android.googlesource.com/kernel/msm-extra/devicetree/+/refs/heads/android-msm-bramble-4.19-android11-qpr1/qcom/lagoon-usb.dtsi#354

>
> It should be OK to include the power-domain also for the PHY node.

Ack.

>
> >  .../bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml          | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> > index 6f31693d9868..3e39e3e0504d 100644
> > --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> > +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> > @@ -17,16 +17,18 @@ properties:
> >    compatible:
> >      enum:
> >        - qcom,sc8280xp-qmp-usb43dp-phy
> > +      - qcom,sm6350-qmp-usb3-dp-phy
> >  
> >    reg:
> >      maxItems: 1
> >  
> >    clocks:
> > -    maxItems: 4
> > +    maxItems: 5
> >  
> >    clock-names:
> >      items:
> >        - const: aux
> > +      - const: cfg_ahb
> >        - const: ref
> >        - const: com_aux
> >        - const: usb3_pipe
>
> So this would need to be moved to an allOf: construct at the end with
> one section each for sc8280xp and sm6350.

Ack.

Thanks for the quick response!

Regards,
Luca

>
> > @@ -61,7 +63,6 @@ required:
> >    - reg
> >    - clocks
> >    - clock-names
> > -  - power-domains
> >    - resets
> >    - reset-names
> >    - vdda-phy-supply
>
> Johan


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RFC PATCH v2 2/3] phy: qcom-qmp-combo: Add config for SM6350

[Johan Hovold]

On Fri, Nov 25, 2022 at 10:27:48AM +0100, Luca Weiss wrote:
> Add the tables and config for the combo phy found on SM6350.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
> @Johan Hovold, here I've added dp_txa & dp_txb, I believe otherwise
> qmp->dp_tx would be wrong. Is this different on sc8280xp or was this a
> mistake on your side? I think this should probably be split out to
> another patch to not mix things up too much.

Yeah, that's a difference in sc8280xp which does not have dedicated TX
registers for DP.

This is probably best handled explicitly when parsing the DT by using
dp_txa/b if they are set and otherwise fallback to txa/txb (e.g.
instead of hiding it in the v5 table by using the same offset in two
places).

It can be done as part of this patch as long as you mention it in the
commit message.

> I think other than that this patch is good.

Indeed, looks good! Nice to see this working out as intended also for
the older platforms.

>  static const struct qmp_phy_init_tbl sm8150_usb3_serdes_tbl[] = {
>  	QMP_PHY_INIT_CFG(QSERDES_V4_COM_SSC_EN_CENTER, 0x01),
>  	QMP_PHY_INIT_CFG(QSERDES_V4_COM_SSC_PER1, 0x31),
> @@ -809,6 +873,8 @@ struct qmp_combo_offsets {
>  	u16 usb3_pcs;
>  	u16 usb3_pcs_usb;
>  	u16 dp_serdes;
> +	u16 dp_txa;
> +	u16 dp_txb;
>  	u16 dp_dp_phy;
>  };
>  
> @@ -975,6 +1041,21 @@ static const char * const sc7180_usb3phy_reset_l[] = {
>  	"phy",
>  };
>  
> +static const struct qmp_combo_offsets qmp_combo_offsets_v3 = {
> +	.com		= 0x0000,
> +	.txa		= 0x1200,
> +	.rxa		= 0x1400,
> +	.txb		= 0x1600,
> +	.rxb		= 0x1800,
> +	.usb3_serdes	= 0x1000,
> +	.usb3_pcs_misc	= 0x1a00,
> +	.usb3_pcs	= 0x1c00,
> +	.dp_serdes	= 0x2000,
> +	.dp_txa		= 0x2200,
> +	.dp_txb		= 0x2600,
> +	.dp_dp_phy	= 0x2c00,
> +};
> +
>  static const struct qmp_combo_offsets qmp_combo_offsets_v5 = {
>  	.com		= 0x0000,
>  	.txa		= 0x0400,

> @@ -2641,8 +2767,8 @@ static int qmp_combo_parse_dt(struct qmp_combo *qmp)
>  	qmp->pcs_usb = base + offs->usb3_pcs_usb;
>  
>  	qmp->dp_serdes = base + offs->dp_serdes;
> -	qmp->dp_tx = base + offs->txa;
> -	qmp->dp_tx2 = base + offs->txb;
> +	qmp->dp_tx = base + offs->dp_txa;
> +	qmp->dp_tx2 = base + offs->dp_txb;
>  	qmp->dp_dp_phy = base + offs->dp_dp_phy;
>  
>  	qmp->pipe_clk = devm_clk_get(dev, "usb3_pipe");

Johan

Re: [RFC PATCH v2 2/3] phy: qcom-qmp-combo: Add config for SM6350

[Johan Hovold]

On Fri, Nov 25, 2022 at 10:27:48AM +0100, Luca Weiss wrote:
> Add the tables and config for the combo phy found on SM6350.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
> @Johan Hovold, here I've added dp_txa & dp_txb, I believe otherwise
> qmp->dp_tx would be wrong. Is this different on sc8280xp or was this a
> mistake on your side? I think this should probably be split out to
> another patch to not mix things up too much.

Yeah, that's a difference in sc8280xp which does not have dedicated TX
registers for DP.

This is probably best handled explicitly when parsing the DT by using
dp_txa/b if they are set and otherwise fallback to txa/txb (e.g.
instead of hiding it in the v5 table by using the same offset in two
places).

It can be done as part of this patch as long as you mention it in the
commit message.

> I think other than that this patch is good.

Indeed, looks good! Nice to see this working out as intended also for
the older platforms.

>  static const struct qmp_phy_init_tbl sm8150_usb3_serdes_tbl[] = {
>  	QMP_PHY_INIT_CFG(QSERDES_V4_COM_SSC_EN_CENTER, 0x01),
>  	QMP_PHY_INIT_CFG(QSERDES_V4_COM_SSC_PER1, 0x31),
> @@ -809,6 +873,8 @@ struct qmp_combo_offsets {
>  	u16 usb3_pcs;
>  	u16 usb3_pcs_usb;
>  	u16 dp_serdes;
> +	u16 dp_txa;
> +	u16 dp_txb;
>  	u16 dp_dp_phy;
>  };
>  
> @@ -975,6 +1041,21 @@ static const char * const sc7180_usb3phy_reset_l[] = {
>  	"phy",
>  };
>  
> +static const struct qmp_combo_offsets qmp_combo_offsets_v3 = {
> +	.com		= 0x0000,
> +	.txa		= 0x1200,
> +	.rxa		= 0x1400,
> +	.txb		= 0x1600,
> +	.rxb		= 0x1800,
> +	.usb3_serdes	= 0x1000,
> +	.usb3_pcs_misc	= 0x1a00,
> +	.usb3_pcs	= 0x1c00,
> +	.dp_serdes	= 0x2000,
> +	.dp_txa		= 0x2200,
> +	.dp_txb		= 0x2600,
> +	.dp_dp_phy	= 0x2c00,
> +};
> +
>  static const struct qmp_combo_offsets qmp_combo_offsets_v5 = {
>  	.com		= 0x0000,
>  	.txa		= 0x0400,

> @@ -2641,8 +2767,8 @@ static int qmp_combo_parse_dt(struct qmp_combo *qmp)
>  	qmp->pcs_usb = base + offs->usb3_pcs_usb;
>  
>  	qmp->dp_serdes = base + offs->dp_serdes;
> -	qmp->dp_tx = base + offs->txa;
> -	qmp->dp_tx2 = base + offs->txb;
> +	qmp->dp_tx = base + offs->dp_txa;
> +	qmp->dp_tx2 = base + offs->dp_txb;
>  	qmp->dp_dp_phy = base + offs->dp_dp_phy;
>  
>  	qmp->pipe_clk = devm_clk_get(dev, "usb3_pipe");

Johan

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RFC PATCH v2 3/3] arm64: dts: qcom: sm6350: Use specific qmpphy compatible

[Johan Hovold]

On Fri, Nov 25, 2022 at 10:27:49AM +0100, Luca Weiss wrote:
> The sc7180 phy compatible works fine for some cases, but it turns out
> sm6350 does need proper phy configuration in the driver, so use the
> newly added sm6350 compatible.
> 
> Because the sm6350 compatible is using the new binding, we need to
> change the node quite a bit to match it.
> 
> This fixes qmpphy init when no USB cable is plugged in during bootloader
> stage.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
> @Johan Hovold, in this patch there's also the question about cfg_ahb,
> power-domains but I'm also not happy about using the
> QMP_USB43DP_USB3_PHY define for the phy reference. Do you think it's a
> good idea to introduce e.g. QMP_USB3DP_USB3_PHY with the same value so
> it's essentially just an alias to the other?

We had that discussion the other week and I believe we agreed that
reusing the define with a more general infix (USB43DP) was fine as it's
just a name for a constant (and the USB43DP constants will be a superset
of the ones needed for USB3-DP PHYs).

> This series is tested on next-20221124 with next branch of linux-phy
> repo (commit bea3ce759b46) merged in.

The dependencies should all be in linux-next as of today.

>  arch/arm64/boot/dts/qcom/sm6350.dtsi | 46 +++++++---------------------
>  1 file changed, 11 insertions(+), 35 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm6350.dtsi b/arch/arm64/boot/dts/qcom/sm6350.dtsi
> index 0f01ff4feb55..923c8bb7e5f8 100644
> --- a/arch/arm64/boot/dts/qcom/sm6350.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm6350.dtsi
> @@ -11,6 +11,7 @@
>  #include <dt-bindings/interconnect/qcom,sm6350.h>
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>  #include <dt-bindings/mailbox/qcom-ipcc.h>
> +#include <dt-bindings/phy/phy-qcom-qmp.h>
>  #include <dt-bindings/power/qcom-rpmpd.h>
>  #include <dt-bindings/soc/qcom,rpmh-rsc.h>
>  
> @@ -1119,50 +1120,25 @@ usb_1_hsphy: phy@88e3000 {
>  			resets = <&gcc GCC_QUSB2PHY_PRIM_BCR>;
>  		};
>  
> -		usb_1_qmpphy: phy@88e9000 {
> -			compatible = "qcom,sc7180-qmp-usb3-dp-phy";
> -			reg = <0 0x088e9000 0 0x200>,
> -			      <0 0x088e8000 0 0x40>,
> -			      <0 0x088ea000 0 0x200>;
> -			status = "disabled";
> -			#address-cells = <2>;
> -			#size-cells = <2>;
> -			ranges;
> +		usb_1_qmpphy: phy@88e8000 {
> +			compatible = "qcom,sm6350-qmp-usb3-dp-phy";
> +			reg = <0 0x088e8000 0 0x3000>;
>  
>  			clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
>  				 <&xo_board>,
>  				 <&rpmhcc RPMH_QLINK_CLK>,
> -				 <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>;
> -			clock-names = "aux", "cfg_ahb", "ref", "com_aux";
> +				 <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
> +				 <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
> +			clock-names = "aux", "cfg_ahb", "ref", "com_aux", "usb3_pipe";

As I mentioned in my reply to the binding, we should double check the
vendor dts and hardware documentation (if possible) before settling on
these names which appears to just have been reused from some older
platform.

>  
>  			resets = <&gcc GCC_USB3_DP_PHY_PRIM_BCR>,
>  				 <&gcc GCC_USB3_PHY_PRIM_BCR>;
>  			reset-names = "phy", "common";
>  
> -			usb_1_ssphy: usb3-phy@88e9200 {
> -				reg = <0 0x088e9200 0 0x200>,
> -				      <0 0x088e9400 0 0x200>,
> -				      <0 0x088e9c00 0 0x400>,
> -				      <0 0x088e9600 0 0x200>,
> -				      <0 0x088e9800 0 0x200>,
> -				      <0 0x088e9a00 0 0x100>;
> -				#clock-cells = <0>;
> -				#phy-cells = <0>;
> -				clocks = <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
> -				clock-names = "pipe0";
> -				clock-output-names = "usb3_phy_pipe_clk_src";
> -			};
> +			#clock-cells = <1>;
> +			#phy-cells = <1>;
>  
> -			dp_phy: dp-phy@88ea200 {
> -				reg = <0 0x088ea200 0 0x200>,
> -				      <0 0x088ea400 0 0x200>,
> -				      <0 0x088eac00 0 0x400>,
> -				      <0 0x088ea600 0 0x200>,
> -				      <0 0x088ea800 0 0x200>,
> -				      <0 0x088eaa00 0 0x100>;

Note that these registers were not correct to begin with and a fix has
been posted here:

	https://lore.kernel.org/all/20221111094729.11842-2-johan+linaro@kernel.org/

Will hopefully show up in linux-next next week so this can be rebased on
top.

> -				#phy-cells = <0>;
> -				#clock-cells = <1>;
> -			};
> +			status = "disabled";
>  		};
>  
>  		dc_noc: interconnect@9160000 {
> @@ -1236,7 +1212,7 @@ usb_1_dwc3: usb@a600000 {
>  				snps,dis_enblslpm_quirk;
>  				snps,has-lpm-erratum;
>  				snps,hird-threshold = /bits/ 8 <0x10>;
> -				phys = <&usb_1_hsphy>, <&usb_1_ssphy>;
> +				phys = <&usb_1_hsphy>, <&usb_1_qmpphy QMP_USB43DP_USB3_PHY>;
>  				phy-names = "usb2-phy", "usb3-phy";
>  			};
>  		};

Johan

Re: [RFC PATCH v2 2/3] phy: qcom-qmp-combo: Add config for SM6350

[Luca Weiss]

Hi Johan,

On Fri Nov 25, 2022 at 11:01 AM CET, Johan Hovold wrote:
> On Fri, Nov 25, 2022 at 10:27:48AM +0100, Luca Weiss wrote:
> > Add the tables and config for the combo phy found on SM6350.
> > 
> > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > ---
> > @Johan Hovold, here I've added dp_txa & dp_txb, I believe otherwise
> > qmp->dp_tx would be wrong. Is this different on sc8280xp or was this a
> > mistake on your side? I think this should probably be split out to
> > another patch to not mix things up too much.
>
> Yeah, that's a difference in sc8280xp which does not have dedicated TX
> registers for DP.

Good to know.

>
> This is probably best handled explicitly when parsing the DT by using
> dp_txa/b if they are set and otherwise fallback to txa/txb (e.g.
> instead of hiding it in the v5 table by using the same offset in two
> places).

Are you thinking about something like this?

if (offs->dp_txa)
    qmp->dp_tx = base + offs->dp_txa
else
    qmp->dp_tx = base + offs->txa;

if (offs->dp_txb)
    qmp->dp_tx2 = base + offs->dp_txb;
else
    qmp->dp_tx2 = base + offs->txb;

This wouldn't handle ".dp_txa = 0x0000" but I don't think this should be
a problem, right?

>
> It can be done as part of this patch as long as you mention it in the
> commit message.

Ack.

Regards
Luca

>
> > I think other than that this patch is good.
>
> Indeed, looks good! Nice to see this working out as intended also for
> the older platforms.
>
> >  static const struct qmp_phy_init_tbl sm8150_usb3_serdes_tbl[] = {
> >  	QMP_PHY_INIT_CFG(QSERDES_V4_COM_SSC_EN_CENTER, 0x01),
> >  	QMP_PHY_INIT_CFG(QSERDES_V4_COM_SSC_PER1, 0x31),
> > @@ -809,6 +873,8 @@ struct qmp_combo_offsets {
> >  	u16 usb3_pcs;
> >  	u16 usb3_pcs_usb;
> >  	u16 dp_serdes;
> > +	u16 dp_txa;
> > +	u16 dp_txb;
> >  	u16 dp_dp_phy;
> >  };
> >  
> > @@ -975,6 +1041,21 @@ static const char * const sc7180_usb3phy_reset_l[] = {
> >  	"phy",
> >  };
> >  
> > +static const struct qmp_combo_offsets qmp_combo_offsets_v3 = {
> > +	.com		= 0x0000,
> > +	.txa		= 0x1200,
> > +	.rxa		= 0x1400,
> > +	.txb		= 0x1600,
> > +	.rxb		= 0x1800,
> > +	.usb3_serdes	= 0x1000,
> > +	.usb3_pcs_misc	= 0x1a00,
> > +	.usb3_pcs	= 0x1c00,
> > +	.dp_serdes	= 0x2000,
> > +	.dp_txa		= 0x2200,
> > +	.dp_txb		= 0x2600,
> > +	.dp_dp_phy	= 0x2c00,
> > +};
> > +
> >  static const struct qmp_combo_offsets qmp_combo_offsets_v5 = {
> >  	.com		= 0x0000,
> >  	.txa		= 0x0400,
>
> > @@ -2641,8 +2767,8 @@ static int qmp_combo_parse_dt(struct qmp_combo *qmp)
> >  	qmp->pcs_usb = base + offs->usb3_pcs_usb;
> >  
> >  	qmp->dp_serdes = base + offs->dp_serdes;
> > -	qmp->dp_tx = base + offs->txa;
> > -	qmp->dp_tx2 = base + offs->txb;
> > +	qmp->dp_tx = base + offs->dp_txa;
> > +	qmp->dp_tx2 = base + offs->dp_txb;
> >  	qmp->dp_dp_phy = base + offs->dp_dp_phy;
> >  
> >  	qmp->pipe_clk = devm_clk_get(dev, "usb3_pipe");
>
> Johan

Re: [RFC PATCH v2 2/3] phy: qcom-qmp-combo: Add config for SM6350

[Luca Weiss]

Hi Johan,

On Fri Nov 25, 2022 at 11:01 AM CET, Johan Hovold wrote:
> On Fri, Nov 25, 2022 at 10:27:48AM +0100, Luca Weiss wrote:
> > Add the tables and config for the combo phy found on SM6350.
> > 
> > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > ---
> > @Johan Hovold, here I've added dp_txa & dp_txb, I believe otherwise
> > qmp->dp_tx would be wrong. Is this different on sc8280xp or was this a
> > mistake on your side? I think this should probably be split out to
> > another patch to not mix things up too much.
>
> Yeah, that's a difference in sc8280xp which does not have dedicated TX
> registers for DP.

Good to know.

>
> This is probably best handled explicitly when parsing the DT by using
> dp_txa/b if they are set and otherwise fallback to txa/txb (e.g.
> instead of hiding it in the v5 table by using the same offset in two
> places).

Are you thinking about something like this?

if (offs->dp_txa)
    qmp->dp_tx = base + offs->dp_txa
else
    qmp->dp_tx = base + offs->txa;

if (offs->dp_txb)
    qmp->dp_tx2 = base + offs->dp_txb;
else
    qmp->dp_tx2 = base + offs->txb;

This wouldn't handle ".dp_txa = 0x0000" but I don't think this should be
a problem, right?

>
> It can be done as part of this patch as long as you mention it in the
> commit message.

Ack.

Regards
Luca

>
> > I think other than that this patch is good.
>
> Indeed, looks good! Nice to see this working out as intended also for
> the older platforms.
>
> >  static const struct qmp_phy_init_tbl sm8150_usb3_serdes_tbl[] = {
> >  	QMP_PHY_INIT_CFG(QSERDES_V4_COM_SSC_EN_CENTER, 0x01),
> >  	QMP_PHY_INIT_CFG(QSERDES_V4_COM_SSC_PER1, 0x31),
> > @@ -809,6 +873,8 @@ struct qmp_combo_offsets {
> >  	u16 usb3_pcs;
> >  	u16 usb3_pcs_usb;
> >  	u16 dp_serdes;
> > +	u16 dp_txa;
> > +	u16 dp_txb;
> >  	u16 dp_dp_phy;
> >  };
> >  
> > @@ -975,6 +1041,21 @@ static const char * const sc7180_usb3phy_reset_l[] = {
> >  	"phy",
> >  };
> >  
> > +static const struct qmp_combo_offsets qmp_combo_offsets_v3 = {
> > +	.com		= 0x0000,
> > +	.txa		= 0x1200,
> > +	.rxa		= 0x1400,
> > +	.txb		= 0x1600,
> > +	.rxb		= 0x1800,
> > +	.usb3_serdes	= 0x1000,
> > +	.usb3_pcs_misc	= 0x1a00,
> > +	.usb3_pcs	= 0x1c00,
> > +	.dp_serdes	= 0x2000,
> > +	.dp_txa		= 0x2200,
> > +	.dp_txb		= 0x2600,
> > +	.dp_dp_phy	= 0x2c00,
> > +};
> > +
> >  static const struct qmp_combo_offsets qmp_combo_offsets_v5 = {
> >  	.com		= 0x0000,
> >  	.txa		= 0x0400,
>
> > @@ -2641,8 +2767,8 @@ static int qmp_combo_parse_dt(struct qmp_combo *qmp)
> >  	qmp->pcs_usb = base + offs->usb3_pcs_usb;
> >  
> >  	qmp->dp_serdes = base + offs->dp_serdes;
> > -	qmp->dp_tx = base + offs->txa;
> > -	qmp->dp_tx2 = base + offs->txb;
> > +	qmp->dp_tx = base + offs->dp_txa;
> > +	qmp->dp_tx2 = base + offs->dp_txb;
> >  	qmp->dp_dp_phy = base + offs->dp_dp_phy;
> >  
> >  	qmp->pipe_clk = devm_clk_get(dev, "usb3_pipe");
>
> Johan


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RFC PATCH v2 1/3] dt-bindings: phy: qcom,qmp-usb3-dp: Add sm6350 compatible

[Johan Hovold]

On Fri, Nov 25, 2022 at 10:55:31AM +0100, Luca Weiss wrote:
> Hi Johan,
> 
> On Fri Nov 25, 2022 at 10:50 AM CET, Johan Hovold wrote:
> > On Fri, Nov 25, 2022 at 10:27:47AM +0100, Luca Weiss wrote:
> > > Add the compatible describing the combo phy found on SM6350.
> > > 
> > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > > ---
> > > @Johan Hovold, I've sent this v2 as RFC because there are several things
> > > where I have questions on how it should be done.
> > > 
> > > In this patch, you can see there's cfg_ahb (&xo_board) and power-domains
> > > is not set. In msm-4.19 &gcc_usb30_prim_gdsc is only used in the
> > > ssusb@a600000 node, or should I also add it to qmpphy?
> >
> > Yeah, you may need to add a platform specific section of the clocks,
> > which appear to be different, even if I'm not sure they are currently
> > described correctly (xo_board as cfg_ahb and "QLINK" as ref). How are
> > they named in the vendor's dts?
> 
> This is the msm-4.19 dts:
> https://android.googlesource.com/kernel/msm-extra/devicetree/+/refs/heads/android-msm-bramble-4.19-android11-qpr1/qcom/lagoon-usb.dtsi#354

		clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
			<&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>,
			<&rpmhcc RPMH_QLINK_CLK>,
			<&gcc GCC_USB3_PRIM_CLKREF_CLK>,
			<&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>;
		clock-names = "aux_clk", "pipe_clk", "ref_clk_src",
				"ref_clk", "com_aux_clk";

So it looks like you don't need update the binding for the clocks as the
above matches sc8280xp:

	aux
	ref
	com_aux
	usb3_pipe

Parent clocks (ref_clk_src) should not be included in the binding, but
rather be handled by the clock driver. For example, see:

	https://lore.kernel.org/all/20221121085058.31213-4-johan+linaro@kernel.org/
	https://lore.kernel.org/all/20221115152956.21677-1-quic_shazhuss@quicinc.com/

> > >  .../bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml          | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> > > index 6f31693d9868..3e39e3e0504d 100644
> > > --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> > > +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> > > @@ -17,16 +17,18 @@ properties:
> > >    compatible:
> > >      enum:
> > >        - qcom,sc8280xp-qmp-usb43dp-phy
> > > +      - qcom,sm6350-qmp-usb3-dp-phy
> > >  
> > >    reg:
> > >      maxItems: 1
> > >  
> > >    clocks:
> > > -    maxItems: 4
> > > +    maxItems: 5
> > >  
> > >    clock-names:
> > >      items:
> > >        - const: aux
> > > +      - const: cfg_ahb
> > >        - const: ref
> > >        - const: com_aux
> > >        - const: usb3_pipe
> >
> > So this would need to be moved to an allOf: construct at the end with
> > one section each for sc8280xp and sm6350.
> 
> Ack.

So no need to change this it seems.

Johan

Re: [RFC PATCH v2 1/3] dt-bindings: phy: qcom,qmp-usb3-dp: Add sm6350 compatible

[Johan Hovold]

On Fri, Nov 25, 2022 at 10:55:31AM +0100, Luca Weiss wrote:
> Hi Johan,
> 
> On Fri Nov 25, 2022 at 10:50 AM CET, Johan Hovold wrote:
> > On Fri, Nov 25, 2022 at 10:27:47AM +0100, Luca Weiss wrote:
> > > Add the compatible describing the combo phy found on SM6350.
> > > 
> > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > > ---
> > > @Johan Hovold, I've sent this v2 as RFC because there are several things
> > > where I have questions on how it should be done.
> > > 
> > > In this patch, you can see there's cfg_ahb (&xo_board) and power-domains
> > > is not set. In msm-4.19 &gcc_usb30_prim_gdsc is only used in the
> > > ssusb@a600000 node, or should I also add it to qmpphy?
> >
> > Yeah, you may need to add a platform specific section of the clocks,
> > which appear to be different, even if I'm not sure they are currently
> > described correctly (xo_board as cfg_ahb and "QLINK" as ref). How are
> > they named in the vendor's dts?
> 
> This is the msm-4.19 dts:
> https://android.googlesource.com/kernel/msm-extra/devicetree/+/refs/heads/android-msm-bramble-4.19-android11-qpr1/qcom/lagoon-usb.dtsi#354

		clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
			<&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>,
			<&rpmhcc RPMH_QLINK_CLK>,
			<&gcc GCC_USB3_PRIM_CLKREF_CLK>,
			<&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>;
		clock-names = "aux_clk", "pipe_clk", "ref_clk_src",
				"ref_clk", "com_aux_clk";

So it looks like you don't need update the binding for the clocks as the
above matches sc8280xp:

	aux
	ref
	com_aux
	usb3_pipe

Parent clocks (ref_clk_src) should not be included in the binding, but
rather be handled by the clock driver. For example, see:

	https://lore.kernel.org/all/20221121085058.31213-4-johan+linaro@kernel.org/
	https://lore.kernel.org/all/20221115152956.21677-1-quic_shazhuss@quicinc.com/

> > >  .../bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml          | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> > > index 6f31693d9868..3e39e3e0504d 100644
> > > --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> > > +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> > > @@ -17,16 +17,18 @@ properties:
> > >    compatible:
> > >      enum:
> > >        - qcom,sc8280xp-qmp-usb43dp-phy
> > > +      - qcom,sm6350-qmp-usb3-dp-phy
> > >  
> > >    reg:
> > >      maxItems: 1
> > >  
> > >    clocks:
> > > -    maxItems: 4
> > > +    maxItems: 5
> > >  
> > >    clock-names:
> > >      items:
> > >        - const: aux
> > > +      - const: cfg_ahb
> > >        - const: ref
> > >        - const: com_aux
> > >        - const: usb3_pipe
> >
> > So this would need to be moved to an allOf: construct at the end with
> > one section each for sc8280xp and sm6350.
> 
> Ack.

So no need to change this it seems.

Johan

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RFC PATCH v2 2/3] phy: qcom-qmp-combo: Add config for SM6350

[Johan Hovold]

On Fri, Nov 25, 2022 at 11:14:53AM +0100, Luca Weiss wrote:
> Hi Johan,
> 
> On Fri Nov 25, 2022 at 11:01 AM CET, Johan Hovold wrote:
> > On Fri, Nov 25, 2022 at 10:27:48AM +0100, Luca Weiss wrote:
> > > Add the tables and config for the combo phy found on SM6350.
> > > 
> > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > > ---
> > > @Johan Hovold, here I've added dp_txa & dp_txb, I believe otherwise
> > > qmp->dp_tx would be wrong. Is this different on sc8280xp or was this a
> > > mistake on your side? I think this should probably be split out to
> > > another patch to not mix things up too much.
> >
> > Yeah, that's a difference in sc8280xp which does not have dedicated TX
> > registers for DP.
> 
> Good to know.
> 
> >
> > This is probably best handled explicitly when parsing the DT by using
> > dp_txa/b if they are set and otherwise fallback to txa/txb (e.g.
> > instead of hiding it in the v5 table by using the same offset in two
> > places).
> 
> Are you thinking about something like this?
> 
> if (offs->dp_txa)
>     qmp->dp_tx = base + offs->dp_txa
> else
>     qmp->dp_tx = base + offs->txa;
> 
> if (offs->dp_txb)
>     qmp->dp_tx2 = base + offs->dp_txb;
> else
>     qmp->dp_tx2 = base + offs->txb;
> 
> This wouldn't handle ".dp_txa = 0x0000" but I don't think this should be
> a problem, right?

Yeah, that should be fine. I'd even merge the branches:

	if (offs->dp_txa) {
		qmp->dp_tx = base + offs->dp_txa;
		qmp->dp_tx2 = base + offs->dp_txb;
	} else {
		qmp->dp_tx = base + offs->txa;
		qmp->dp_tx2 = base + offs->txb;
	}

Johan

Re: [RFC PATCH v2 2/3] phy: qcom-qmp-combo: Add config for SM6350

[Johan Hovold]

On Fri, Nov 25, 2022 at 11:14:53AM +0100, Luca Weiss wrote:
> Hi Johan,
> 
> On Fri Nov 25, 2022 at 11:01 AM CET, Johan Hovold wrote:
> > On Fri, Nov 25, 2022 at 10:27:48AM +0100, Luca Weiss wrote:
> > > Add the tables and config for the combo phy found on SM6350.
> > > 
> > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > > ---
> > > @Johan Hovold, here I've added dp_txa & dp_txb, I believe otherwise
> > > qmp->dp_tx would be wrong. Is this different on sc8280xp or was this a
> > > mistake on your side? I think this should probably be split out to
> > > another patch to not mix things up too much.
> >
> > Yeah, that's a difference in sc8280xp which does not have dedicated TX
> > registers for DP.
> 
> Good to know.
> 
> >
> > This is probably best handled explicitly when parsing the DT by using
> > dp_txa/b if they are set and otherwise fallback to txa/txb (e.g.
> > instead of hiding it in the v5 table by using the same offset in two
> > places).
> 
> Are you thinking about something like this?
> 
> if (offs->dp_txa)
>     qmp->dp_tx = base + offs->dp_txa
> else
>     qmp->dp_tx = base + offs->txa;
> 
> if (offs->dp_txb)
>     qmp->dp_tx2 = base + offs->dp_txb;
> else
>     qmp->dp_tx2 = base + offs->txb;
> 
> This wouldn't handle ".dp_txa = 0x0000" but I don't think this should be
> a problem, right?

Yeah, that should be fine. I'd even merge the branches:

	if (offs->dp_txa) {
		qmp->dp_tx = base + offs->dp_txa;
		qmp->dp_tx2 = base + offs->dp_txb;
	} else {
		qmp->dp_tx = base + offs->txa;
		qmp->dp_tx2 = base + offs->txb;
	}

Johan

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RFC PATCH v2 1/3] dt-bindings: phy: qcom,qmp-usb3-dp: Add sm6350 compatible

[Luca Weiss]

On Fri Nov 25, 2022 at 11:19 AM CET, Johan Hovold wrote:
> On Fri, Nov 25, 2022 at 10:55:31AM +0100, Luca Weiss wrote:
> > Hi Johan,
> > 
> > On Fri Nov 25, 2022 at 10:50 AM CET, Johan Hovold wrote:
> > > On Fri, Nov 25, 2022 at 10:27:47AM +0100, Luca Weiss wrote:
> > > > Add the compatible describing the combo phy found on SM6350.
> > > > 
> > > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > > > ---
> > > > @Johan Hovold, I've sent this v2 as RFC because there are several things
> > > > where I have questions on how it should be done.
> > > > 
> > > > In this patch, you can see there's cfg_ahb (&xo_board) and power-domains
> > > > is not set. In msm-4.19 &gcc_usb30_prim_gdsc is only used in the
> > > > ssusb@a600000 node, or should I also add it to qmpphy?
> > >
> > > Yeah, you may need to add a platform specific section of the clocks,
> > > which appear to be different, even if I'm not sure they are currently
> > > described correctly (xo_board as cfg_ahb and "QLINK" as ref). How are
> > > they named in the vendor's dts?
> > 
> > This is the msm-4.19 dts:
> > https://android.googlesource.com/kernel/msm-extra/devicetree/+/refs/heads/android-msm-bramble-4.19-android11-qpr1/qcom/lagoon-usb.dtsi#354
>
> 		clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
> 			<&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>,
> 			<&rpmhcc RPMH_QLINK_CLK>,
> 			<&gcc GCC_USB3_PRIM_CLKREF_CLK>,
> 			<&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>;
> 		clock-names = "aux_clk", "pipe_clk", "ref_clk_src",
> 				"ref_clk", "com_aux_clk";
>
> So it looks like you don't need update the binding for the clocks as the
> above matches sc8280xp:
>
> 	aux
> 	ref
> 	com_aux
> 	usb3_pipe

Thanks for checking!

>
> Parent clocks (ref_clk_src) should not be included in the binding, but
> rather be handled by the clock driver. For example, see:
>
> 	https://lore.kernel.org/all/20221121085058.31213-4-johan+linaro@kernel.org/
> 	https://lore.kernel.org/all/20221115152956.21677-1-quic_shazhuss@quicinc.com/

So I assume you mean that I shouldn't do this:

clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
     <&rpmhcc RPMH_QLINK_CLK>,
     <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
     <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
clock-names = "aux", "ref", "com_aux", "usb3_pipe";

But for "ref" use GCC_USB3_PRIM_CLKREF_CLK? That also seems to work
fine, also if RPMH_QLINK_CLK is not used from Linux-side (checked in
debugfs).


And for the driver patch, I've discovered that this phy doesn't have
separate txa/tbx region, so dts was also wrong there. Do you know if
there's a way to test DP phy initialization without having all the USB-C
plumbing in place? Might be good to validate at least phy init works if
we're already touching all of this.

Regards
Luca

>
> > > >  .../bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml          | 5 +++--
> > > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> > > > index 6f31693d9868..3e39e3e0504d 100644
> > > > --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> > > > +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> > > > @@ -17,16 +17,18 @@ properties:
> > > >    compatible:
> > > >      enum:
> > > >        - qcom,sc8280xp-qmp-usb43dp-phy
> > > > +      - qcom,sm6350-qmp-usb3-dp-phy
> > > >  
> > > >    reg:
> > > >      maxItems: 1
> > > >  
> > > >    clocks:
> > > > -    maxItems: 4
> > > > +    maxItems: 5
> > > >  
> > > >    clock-names:
> > > >      items:
> > > >        - const: aux
> > > > +      - const: cfg_ahb
> > > >        - const: ref
> > > >        - const: com_aux
> > > >        - const: usb3_pipe
> > >
> > > So this would need to be moved to an allOf: construct at the end with
> > > one section each for sc8280xp and sm6350.
> > 
> > Ack.
>
> So no need to change this it seems.
>
> Johan

Re: [RFC PATCH v2 1/3] dt-bindings: phy: qcom,qmp-usb3-dp: Add sm6350 compatible

[Luca Weiss]

On Fri Nov 25, 2022 at 11:19 AM CET, Johan Hovold wrote:
> On Fri, Nov 25, 2022 at 10:55:31AM +0100, Luca Weiss wrote:
> > Hi Johan,
> > 
> > On Fri Nov 25, 2022 at 10:50 AM CET, Johan Hovold wrote:
> > > On Fri, Nov 25, 2022 at 10:27:47AM +0100, Luca Weiss wrote:
> > > > Add the compatible describing the combo phy found on SM6350.
> > > > 
> > > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > > > ---
> > > > @Johan Hovold, I've sent this v2 as RFC because there are several things
> > > > where I have questions on how it should be done.
> > > > 
> > > > In this patch, you can see there's cfg_ahb (&xo_board) and power-domains
> > > > is not set. In msm-4.19 &gcc_usb30_prim_gdsc is only used in the
> > > > ssusb@a600000 node, or should I also add it to qmpphy?
> > >
> > > Yeah, you may need to add a platform specific section of the clocks,
> > > which appear to be different, even if I'm not sure they are currently
> > > described correctly (xo_board as cfg_ahb and "QLINK" as ref). How are
> > > they named in the vendor's dts?
> > 
> > This is the msm-4.19 dts:
> > https://android.googlesource.com/kernel/msm-extra/devicetree/+/refs/heads/android-msm-bramble-4.19-android11-qpr1/qcom/lagoon-usb.dtsi#354
>
> 		clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
> 			<&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>,
> 			<&rpmhcc RPMH_QLINK_CLK>,
> 			<&gcc GCC_USB3_PRIM_CLKREF_CLK>,
> 			<&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>;
> 		clock-names = "aux_clk", "pipe_clk", "ref_clk_src",
> 				"ref_clk", "com_aux_clk";
>
> So it looks like you don't need update the binding for the clocks as the
> above matches sc8280xp:
>
> 	aux
> 	ref
> 	com_aux
> 	usb3_pipe

Thanks for checking!

>
> Parent clocks (ref_clk_src) should not be included in the binding, but
> rather be handled by the clock driver. For example, see:
>
> 	https://lore.kernel.org/all/20221121085058.31213-4-johan+linaro@kernel.org/
> 	https://lore.kernel.org/all/20221115152956.21677-1-quic_shazhuss@quicinc.com/

So I assume you mean that I shouldn't do this:

clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
     <&rpmhcc RPMH_QLINK_CLK>,
     <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
     <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
clock-names = "aux", "ref", "com_aux", "usb3_pipe";

But for "ref" use GCC_USB3_PRIM_CLKREF_CLK? That also seems to work
fine, also if RPMH_QLINK_CLK is not used from Linux-side (checked in
debugfs).


And for the driver patch, I've discovered that this phy doesn't have
separate txa/tbx region, so dts was also wrong there. Do you know if
there's a way to test DP phy initialization without having all the USB-C
plumbing in place? Might be good to validate at least phy init works if
we're already touching all of this.

Regards
Luca

>
> > > >  .../bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml          | 5 +++--
> > > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> > > > index 6f31693d9868..3e39e3e0504d 100644
> > > > --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> > > > +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> > > > @@ -17,16 +17,18 @@ properties:
> > > >    compatible:
> > > >      enum:
> > > >        - qcom,sc8280xp-qmp-usb43dp-phy
> > > > +      - qcom,sm6350-qmp-usb3-dp-phy
> > > >  
> > > >    reg:
> > > >      maxItems: 1
> > > >  
> > > >    clocks:
> > > > -    maxItems: 4
> > > > +    maxItems: 5
> > > >  
> > > >    clock-names:
> > > >      items:
> > > >        - const: aux
> > > > +      - const: cfg_ahb
> > > >        - const: ref
> > > >        - const: com_aux
> > > >        - const: usb3_pipe
> > >
> > > So this would need to be moved to an allOf: construct at the end with
> > > one section each for sc8280xp and sm6350.
> > 
> > Ack.
>
> So no need to change this it seems.
>
> Johan


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RFC PATCH v2 1/3] dt-bindings: phy: qcom,qmp-usb3-dp: Add sm6350 compatible

[Johan Hovold]

On Fri, Nov 25, 2022 at 01:53:25PM +0100, Luca Weiss wrote:
> On Fri Nov 25, 2022 at 11:19 AM CET, Johan Hovold wrote:
> > On Fri, Nov 25, 2022 at 10:55:31AM +0100, Luca Weiss wrote:
> > > On Fri Nov 25, 2022 at 10:50 AM CET, Johan Hovold wrote:

> > > > Yeah, you may need to add a platform specific section of the clocks,
> > > > which appear to be different, even if I'm not sure they are currently
> > > > described correctly (xo_board as cfg_ahb and "QLINK" as ref). How are
> > > > they named in the vendor's dts?
> > > 
> > > This is the msm-4.19 dts:
> > > https://android.googlesource.com/kernel/msm-extra/devicetree/+/refs/heads/android-msm-bramble-4.19-android11-qpr1/qcom/lagoon-usb.dtsi#354
> >
> > 		clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
> > 			<&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>,
> > 			<&rpmhcc RPMH_QLINK_CLK>,
> > 			<&gcc GCC_USB3_PRIM_CLKREF_CLK>,
> > 			<&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>;
> > 		clock-names = "aux_clk", "pipe_clk", "ref_clk_src",
> > 				"ref_clk", "com_aux_clk";
> >
> > So it looks like you don't need update the binding for the clocks as the
> > above matches sc8280xp:
> >
> > 	aux
> > 	ref
> > 	com_aux
> > 	usb3_pipe
> 
> Thanks for checking!
> 
> >
> > Parent clocks (ref_clk_src) should not be included in the binding, but
> > rather be handled by the clock driver. For example, see:
> >
> > 	https://lore.kernel.org/all/20221121085058.31213-4-johan+linaro@kernel.org/
> > 	https://lore.kernel.org/all/20221115152956.21677-1-quic_shazhuss@quicinc.com/
> 
> So I assume you mean that I shouldn't do this:
> 
> clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
>      <&rpmhcc RPMH_QLINK_CLK>,
>      <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
>      <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
> clock-names = "aux", "ref", "com_aux", "usb3_pipe";
> 
> But for "ref" use GCC_USB3_PRIM_CLKREF_CLK? That also seems to work
> fine, also if RPMH_QLINK_CLK is not used from Linux-side (checked in
> debugfs).

Exactly. Since the vendor dts describes RPMH_QLINK_CLK as parent of ref,
I'd suggest modelling that in the clock driver. Perhaps it has just been
left on by the boot firmware. Someone with access to docs may be able
explain how it is supposed to be used.

> And for the driver patch, I've discovered that this phy doesn't have
> separate txa/tbx region, so dts was also wrong there. Do you know if
> there's a way to test DP phy initialization without having all the USB-C
> plumbing in place? Might be good to validate at least phy init works if
> we're already touching all of this.

Do you mean that it appears to work as sc8280xp with txa/txb shared by
both the USB and DP parts?

I guess you need a proper setup to test it properly. Not sure what
you'll be able to learn otherwise, apart from whether it passes basic
smoke testing.

Johan

Re: [RFC PATCH v2 1/3] dt-bindings: phy: qcom,qmp-usb3-dp: Add sm6350 compatible

[Johan Hovold]

On Fri, Nov 25, 2022 at 01:53:25PM +0100, Luca Weiss wrote:
> On Fri Nov 25, 2022 at 11:19 AM CET, Johan Hovold wrote:
> > On Fri, Nov 25, 2022 at 10:55:31AM +0100, Luca Weiss wrote:
> > > On Fri Nov 25, 2022 at 10:50 AM CET, Johan Hovold wrote:

> > > > Yeah, you may need to add a platform specific section of the clocks,
> > > > which appear to be different, even if I'm not sure they are currently
> > > > described correctly (xo_board as cfg_ahb and "QLINK" as ref). How are
> > > > they named in the vendor's dts?
> > > 
> > > This is the msm-4.19 dts:
> > > https://android.googlesource.com/kernel/msm-extra/devicetree/+/refs/heads/android-msm-bramble-4.19-android11-qpr1/qcom/lagoon-usb.dtsi#354
> >
> > 		clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
> > 			<&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>,
> > 			<&rpmhcc RPMH_QLINK_CLK>,
> > 			<&gcc GCC_USB3_PRIM_CLKREF_CLK>,
> > 			<&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>;
> > 		clock-names = "aux_clk", "pipe_clk", "ref_clk_src",
> > 				"ref_clk", "com_aux_clk";
> >
> > So it looks like you don't need update the binding for the clocks as the
> > above matches sc8280xp:
> >
> > 	aux
> > 	ref
> > 	com_aux
> > 	usb3_pipe
> 
> Thanks for checking!
> 
> >
> > Parent clocks (ref_clk_src) should not be included in the binding, but
> > rather be handled by the clock driver. For example, see:
> >
> > 	https://lore.kernel.org/all/20221121085058.31213-4-johan+linaro@kernel.org/
> > 	https://lore.kernel.org/all/20221115152956.21677-1-quic_shazhuss@quicinc.com/
> 
> So I assume you mean that I shouldn't do this:
> 
> clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
>      <&rpmhcc RPMH_QLINK_CLK>,
>      <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
>      <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
> clock-names = "aux", "ref", "com_aux", "usb3_pipe";
> 
> But for "ref" use GCC_USB3_PRIM_CLKREF_CLK? That also seems to work
> fine, also if RPMH_QLINK_CLK is not used from Linux-side (checked in
> debugfs).

Exactly. Since the vendor dts describes RPMH_QLINK_CLK as parent of ref,
I'd suggest modelling that in the clock driver. Perhaps it has just been
left on by the boot firmware. Someone with access to docs may be able
explain how it is supposed to be used.

> And for the driver patch, I've discovered that this phy doesn't have
> separate txa/tbx region, so dts was also wrong there. Do you know if
> there's a way to test DP phy initialization without having all the USB-C
> plumbing in place? Might be good to validate at least phy init works if
> we're already touching all of this.

Do you mean that it appears to work as sc8280xp with txa/txb shared by
both the USB and DP parts?

I guess you need a proper setup to test it properly. Not sure what
you'll be able to learn otherwise, apart from whether it passes basic
smoke testing.

Johan

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RFC PATCH v2 1/3] dt-bindings: phy: qcom,qmp-usb3-dp: Add sm6350 compatible

[Luca Weiss]

On Fri Nov 25, 2022 at 2:52 PM CET, Johan Hovold wrote:
> On Fri, Nov 25, 2022 at 01:53:25PM +0100, Luca Weiss wrote:
> > > Parent clocks (ref_clk_src) should not be included in the binding, but
> > > rather be handled by the clock driver. For example, see:
> > >
> > > 	https://lore.kernel.org/all/20221121085058.31213-4-johan+linaro@kernel.org/
> > > 	https://lore.kernel.org/all/20221115152956.21677-1-quic_shazhuss@quicinc.com/
> > 
> > So I assume you mean that I shouldn't do this:
> > 
> > clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
> >      <&rpmhcc RPMH_QLINK_CLK>,
> >      <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
> >      <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
> > clock-names = "aux", "ref", "com_aux", "usb3_pipe";
> > 
> > But for "ref" use GCC_USB3_PRIM_CLKREF_CLK? That also seems to work
> > fine, also if RPMH_QLINK_CLK is not used from Linux-side (checked in
> > debugfs).
>
> Exactly. Since the vendor dts describes RPMH_QLINK_CLK as parent of ref,
> I'd suggest modelling that in the clock driver. Perhaps it has just been
> left on by the boot firmware. Someone with access to docs may be able
> explain how it is supposed to be used.

RPMH_QLINK_CLK is also in msm-4.19 ref_clk_src for
GCC_UFS_MEM_CLKREF_CLK (ufsphy_mem) and also ref_clk (ufshc_mem).

Honestly since it works fine without adding this to gcc driver and I
don't really know much about clk (and have no docs for this) would it be
okay to just ignore RPMH_QLINK_CLK?

>
> > And for the driver patch, I've discovered that this phy doesn't have
> > separate txa/tbx region, so dts was also wrong there. Do you know if
> > there's a way to test DP phy initialization without having all the USB-C
> > plumbing in place? Might be good to validate at least phy init works if
> > we're already touching all of this.
>
> Do you mean that it appears to work as sc8280xp with txa/txb shared by
> both the USB and DP parts?

Yes, looks like it. Can't find any evidence pointing in any other
direction at least, everything I've seen shows .txa = 0x1200 & .txb =
0x1600.

>
> I guess you need a proper setup to test it properly. Not sure what
> you'll be able to learn otherwise, apart from whether it passes basic
> smoke testing.

Currently it's not even smoke testing because dp phy is never getting
enabled because there's no consumer. That's why I guess it was never
noticed it's wrongly described in dts.

Regards
Luca

>
> Johan

Re: [RFC PATCH v2 1/3] dt-bindings: phy: qcom,qmp-usb3-dp: Add sm6350 compatible

[Luca Weiss]

On Fri Nov 25, 2022 at 2:52 PM CET, Johan Hovold wrote:
> On Fri, Nov 25, 2022 at 01:53:25PM +0100, Luca Weiss wrote:
> > > Parent clocks (ref_clk_src) should not be included in the binding, but
> > > rather be handled by the clock driver. For example, see:
> > >
> > > 	https://lore.kernel.org/all/20221121085058.31213-4-johan+linaro@kernel.org/
> > > 	https://lore.kernel.org/all/20221115152956.21677-1-quic_shazhuss@quicinc.com/
> > 
> > So I assume you mean that I shouldn't do this:
> > 
> > clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
> >      <&rpmhcc RPMH_QLINK_CLK>,
> >      <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
> >      <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
> > clock-names = "aux", "ref", "com_aux", "usb3_pipe";
> > 
> > But for "ref" use GCC_USB3_PRIM_CLKREF_CLK? That also seems to work
> > fine, also if RPMH_QLINK_CLK is not used from Linux-side (checked in
> > debugfs).
>
> Exactly. Since the vendor dts describes RPMH_QLINK_CLK as parent of ref,
> I'd suggest modelling that in the clock driver. Perhaps it has just been
> left on by the boot firmware. Someone with access to docs may be able
> explain how it is supposed to be used.

RPMH_QLINK_CLK is also in msm-4.19 ref_clk_src for
GCC_UFS_MEM_CLKREF_CLK (ufsphy_mem) and also ref_clk (ufshc_mem).

Honestly since it works fine without adding this to gcc driver and I
don't really know much about clk (and have no docs for this) would it be
okay to just ignore RPMH_QLINK_CLK?

>
> > And for the driver patch, I've discovered that this phy doesn't have
> > separate txa/tbx region, so dts was also wrong there. Do you know if
> > there's a way to test DP phy initialization without having all the USB-C
> > plumbing in place? Might be good to validate at least phy init works if
> > we're already touching all of this.
>
> Do you mean that it appears to work as sc8280xp with txa/txb shared by
> both the USB and DP parts?

Yes, looks like it. Can't find any evidence pointing in any other
direction at least, everything I've seen shows .txa = 0x1200 & .txb =
0x1600.

>
> I guess you need a proper setup to test it properly. Not sure what
> you'll be able to learn otherwise, apart from whether it passes basic
> smoke testing.

Currently it's not even smoke testing because dp phy is never getting
enabled because there's no consumer. That's why I guess it was never
noticed it's wrongly described in dts.

Regards
Luca

>
> Johan


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RFC PATCH v2 1/3] dt-bindings: phy: qcom,qmp-usb3-dp: Add sm6350 compatible

[Johan Hovold]

On Fri, Nov 25, 2022 at 03:12:24PM +0100, Luca Weiss wrote:
> On Fri Nov 25, 2022 at 2:52 PM CET, Johan Hovold wrote:
> > On Fri, Nov 25, 2022 at 01:53:25PM +0100, Luca Weiss wrote:
> > > > Parent clocks (ref_clk_src) should not be included in the binding, but
> > > > rather be handled by the clock driver. For example, see:
> > > >
> > > > 	https://lore.kernel.org/all/20221121085058.31213-4-johan+linaro@kernel.org/
> > > > 	https://lore.kernel.org/all/20221115152956.21677-1-quic_shazhuss@quicinc.com/
> > > 
> > > So I assume you mean that I shouldn't do this:
> > > 
> > > clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
> > >      <&rpmhcc RPMH_QLINK_CLK>,
> > >      <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
> > >      <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
> > > clock-names = "aux", "ref", "com_aux", "usb3_pipe";
> > > 
> > > But for "ref" use GCC_USB3_PRIM_CLKREF_CLK? That also seems to work
> > > fine, also if RPMH_QLINK_CLK is not used from Linux-side (checked in
> > > debugfs).
> >
> > Exactly. Since the vendor dts describes RPMH_QLINK_CLK as parent of ref,
> > I'd suggest modelling that in the clock driver. Perhaps it has just been
> > left on by the boot firmware. Someone with access to docs may be able
> > explain how it is supposed to be used.
> 
> RPMH_QLINK_CLK is also in msm-4.19 ref_clk_src for
> GCC_UFS_MEM_CLKREF_CLK (ufsphy_mem) and also ref_clk (ufshc_mem).
> 
> Honestly since it works fine without adding this to gcc driver and I
> don't really know much about clk (and have no docs for this) would it be
> okay to just ignore RPMH_QLINK_CLK?

Preferably it should be fixed now as it may be harder to figure out
what's missing in case this causes trouble in some setup later.

But, yeah, the lack of documentation is a pain.

Hopefully Bjorn or Vinod can help out with getting this sorted properly.

> > > And for the driver patch, I've discovered that this phy doesn't have
> > > separate txa/tbx region, so dts was also wrong there. Do you know if
> > > there's a way to test DP phy initialization without having all the USB-C
> > > plumbing in place? Might be good to validate at least phy init works if
> > > we're already touching all of this.
> >
> > Do you mean that it appears to work as sc8280xp with txa/txb shared by
> > both the USB and DP parts?
> 
> Yes, looks like it. Can't find any evidence pointing in any other
> direction at least, everything I've seen shows .txa = 0x1200 & .txb =
> 0x1600.

Ok. I've also only seen indirect references to the DP registers
for the older platforms, but at least of them do have the separate DP TX
regions.

> > I guess you need a proper setup to test it properly. Not sure what
> > you'll be able to learn otherwise, apart from whether it passes basic
> > smoke testing.
> 
> Currently it's not even smoke testing because dp phy is never getting
> enabled because there's no consumer. That's why I guess it was never
> noticed it's wrongly described in dts.

Yeah, people shouldn't be adding (copy-pasted) nodes for peripherals
that they are not able to test (especially given the lack of
documentation), but I guess the USB3-DP case is a bit of a grey area as
the USB part can have been verified. Fortunately, this should be less of
any issue with the new binding scheme.

Johan

Re: [RFC PATCH v2 1/3] dt-bindings: phy: qcom,qmp-usb3-dp: Add sm6350 compatible

[Johan Hovold]

On Fri, Nov 25, 2022 at 03:12:24PM +0100, Luca Weiss wrote:
> On Fri Nov 25, 2022 at 2:52 PM CET, Johan Hovold wrote:
> > On Fri, Nov 25, 2022 at 01:53:25PM +0100, Luca Weiss wrote:
> > > > Parent clocks (ref_clk_src) should not be included in the binding, but
> > > > rather be handled by the clock driver. For example, see:
> > > >
> > > > 	https://lore.kernel.org/all/20221121085058.31213-4-johan+linaro@kernel.org/
> > > > 	https://lore.kernel.org/all/20221115152956.21677-1-quic_shazhuss@quicinc.com/
> > > 
> > > So I assume you mean that I shouldn't do this:
> > > 
> > > clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
> > >      <&rpmhcc RPMH_QLINK_CLK>,
> > >      <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
> > >      <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
> > > clock-names = "aux", "ref", "com_aux", "usb3_pipe";
> > > 
> > > But for "ref" use GCC_USB3_PRIM_CLKREF_CLK? That also seems to work
> > > fine, also if RPMH_QLINK_CLK is not used from Linux-side (checked in
> > > debugfs).
> >
> > Exactly. Since the vendor dts describes RPMH_QLINK_CLK as parent of ref,
> > I'd suggest modelling that in the clock driver. Perhaps it has just been
> > left on by the boot firmware. Someone with access to docs may be able
> > explain how it is supposed to be used.
> 
> RPMH_QLINK_CLK is also in msm-4.19 ref_clk_src for
> GCC_UFS_MEM_CLKREF_CLK (ufsphy_mem) and also ref_clk (ufshc_mem).
> 
> Honestly since it works fine without adding this to gcc driver and I
> don't really know much about clk (and have no docs for this) would it be
> okay to just ignore RPMH_QLINK_CLK?

Preferably it should be fixed now as it may be harder to figure out
what's missing in case this causes trouble in some setup later.

But, yeah, the lack of documentation is a pain.

Hopefully Bjorn or Vinod can help out with getting this sorted properly.

> > > And for the driver patch, I've discovered that this phy doesn't have
> > > separate txa/tbx region, so dts was also wrong there. Do you know if
> > > there's a way to test DP phy initialization without having all the USB-C
> > > plumbing in place? Might be good to validate at least phy init works if
> > > we're already touching all of this.
> >
> > Do you mean that it appears to work as sc8280xp with txa/txb shared by
> > both the USB and DP parts?
> 
> Yes, looks like it. Can't find any evidence pointing in any other
> direction at least, everything I've seen shows .txa = 0x1200 & .txb =
> 0x1600.

Ok. I've also only seen indirect references to the DP registers
for the older platforms, but at least of them do have the separate DP TX
regions.

> > I guess you need a proper setup to test it properly. Not sure what
> > you'll be able to learn otherwise, apart from whether it passes basic
> > smoke testing.
> 
> Currently it's not even smoke testing because dp phy is never getting
> enabled because there's no consumer. That's why I guess it was never
> noticed it's wrongly described in dts.

Yeah, people shouldn't be adding (copy-pasted) nodes for peripherals
that they are not able to test (especially given the lack of
documentation), but I guess the USB3-DP case is a bit of a grey area as
the USB part can have been verified. Fortunately, this should be less of
any issue with the new binding scheme.

Johan

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy