[PATCH v2 0/3] phy: qcom-qmp-pcie: Add support to keep refclk always on

[PATCH v2 0/3] phy: qcom-qmp-pcie: Add support to keep refclk always on

[Krishna chaitanya chundru]

This series adds support to provide refclk to endpoint even in low
power states.

Due to some platform specific issues with CLKREQ signal, it is not being
propagated to the host and as host doesn't know the clkreq signal host is
not sending refclk. Due to this endpoint is seeing linkdown and going
to bad state.
To avoid those ref clk should be provided always to the endpoint. The
issue is coming only when ep intiates the L1.1 or L1.2 exit and clkreq
is not being propagated properly to the host. 

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
Changes in v2:
- Added refclk cntrl registers to the applicable phy versions & added reg layout where
- refclk cntrl offset needs to be updated (Dmitry)
- Error out if refclk_always_on is set and there is no refclk control register to enable it (Dmitry)
- updated the dt-binding description & some nit's as suggested by (Bjorn)
- Link to v1: https://lore.kernel.org/r/20231106-refclk_always_on-v1-0-17a7fd8b532b@quicinc.com

---
Krishna chaitanya chundru (3):
      dt-bindings: phy: qcom,qmp: Add PCIe qcom,refclk-always-on property
      phy: qcom-qmp-pcie: Add endpoint refclk control register offset
      phy: qcom-qmp-pcie: Add support for keeping refclk always on

 .../bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml   |  7 ++++
 drivers/phy/qualcomm/phy-qcom-qmp-pcie.c           | 40 ++++++++++++++++++++--
 drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h    |  1 +
 drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h |  1 +
 drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h |  1 +
 5 files changed, 47 insertions(+), 3 deletions(-)
---
base-commit: 71e68e182e382e951d6248bccc3c960dcec5a718
change-id: 20231106-refclk_always_on-9beae8297cb8

Best regards,
-- 
Krishna chaitanya chundru <quic_krichai@quicinc.com>

[PATCH v2 1/3] dt-bindings: phy: qcom,qmp: Add PCIe qcom,refclk-always-on property

[Krishna chaitanya chundru]

Document qcom,refclk-always-on property which is needed in some platforms
to supply refclk even in PCIe low power states.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 .../devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml        | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
index 2c3d6553a7ba..263291447a5b 100644
--- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
@@ -93,6 +93,13 @@ properties:
   "#phy-cells":
     const: 0
 
+  qcom,refclk-always-on:
+    type: boolean
+    description: If there is some issues in platform with clkreq signal
+      propagation to the host and due to that host will not send refclk, which
+      results in linkdown in L1.2 or L1.1 exit initiated by EP. This property
+      if set keeps refclk always on even in Low power states (optional).
+
 required:
   - compatible
   - reg

-- 
2.42.0

[PATCH v2 2/3] phy: qcom-qmp-pcie: Add endpoint refclk control register offset

[Krishna chaitanya chundru]

Some platforms needs to keep endpoint refclk always on, for this
purpose add this offset for all the applicable phy versions.

And also add reg layout for few controllers as we are adding
endpoint refclk control register which changes based upon phy version.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-pcie.c           | 26 +++++++++++++++++++---
 drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h    |  1 +
 drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h |  1 +
 drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h |  1 +
 4 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
index a63ca7424974..74d03d217ff2 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
@@ -77,6 +77,7 @@ enum qphy_reg_layout {
 	QPHY_START_CTRL,
 	QPHY_PCS_STATUS,
 	QPHY_PCS_POWER_DOWN_CONTROL,
+	QPHY_PCS_ENDPOINT_REFCLK_CNTRL,
 	/* Keep last to ensure regs_layout arrays are properly initialized */
 	QPHY_LAYOUT_SIZE
 };
@@ -93,6 +94,7 @@ static const unsigned int pciephy_v3_regs_layout[QPHY_LAYOUT_SIZE] = {
 	[QPHY_START_CTRL]		= QPHY_V3_PCS_START_CONTROL,
 	[QPHY_PCS_STATUS]		= QPHY_V3_PCS_PCS_STATUS,
 	[QPHY_PCS_POWER_DOWN_CONTROL]	= QPHY_V3_PCS_POWER_DOWN_CONTROL,
+	[QPHY_PCS_ENDPOINT_REFCLK_CNTRL]	= QPHY_V3_PCS_ENDPOINT_REFCLK_CNTRL,
 };
 
 static const unsigned int sdm845_qhp_pciephy_regs_layout[QPHY_LAYOUT_SIZE] = {
@@ -107,6 +109,7 @@ static const unsigned int pciephy_v4_regs_layout[QPHY_LAYOUT_SIZE] = {
 	[QPHY_START_CTRL]		= QPHY_V4_PCS_START_CONTROL,
 	[QPHY_PCS_STATUS]		= QPHY_V4_PCS_PCS_STATUS1,
 	[QPHY_PCS_POWER_DOWN_CONTROL]	= QPHY_V4_PCS_POWER_DOWN_CONTROL,
+	[QPHY_PCS_ENDPOINT_REFCLK_CNTRL]	= QPHY_V4_PCS_PCIE_ENDPOINT_REFCLK_CNTRL,
 };
 
 static const unsigned int pciephy_v5_regs_layout[QPHY_LAYOUT_SIZE] = {
@@ -114,6 +117,23 @@ static const unsigned int pciephy_v5_regs_layout[QPHY_LAYOUT_SIZE] = {
 	[QPHY_START_CTRL]		= QPHY_V5_PCS_START_CONTROL,
 	[QPHY_PCS_STATUS]		= QPHY_V5_PCS_PCS_STATUS1,
 	[QPHY_PCS_POWER_DOWN_CONTROL]	= QPHY_V5_PCS_POWER_DOWN_CONTROL,
+	[QPHY_PCS_ENDPOINT_REFCLK_CNTRL]	= QPHY_V5_PCS_PCIE_ENDPOINT_REFCLK_CNTRL,
+};
+
+static const unsigned int pciephy_v5_20_regs_layout[QPHY_LAYOUT_SIZE] = {
+	[QPHY_SW_RESET]			= QPHY_V5_PCS_SW_RESET,
+	[QPHY_START_CTRL]		= QPHY_V5_PCS_START_CONTROL,
+	[QPHY_PCS_STATUS]		= QPHY_V5_PCS_PCS_STATUS1,
+	[QPHY_PCS_POWER_DOWN_CONTROL]	= QPHY_V5_PCS_POWER_DOWN_CONTROL,
+	[QPHY_PCS_ENDPOINT_REFCLK_CNTRL]	= QPHY_V5_20_PCS_PCIE_ENDPOINT_REFCLK_CNTRL,
+};
+
+static const unsigned int pciephy_v6_20_regs_layout[QPHY_LAYOUT_SIZE] = {
+	[QPHY_SW_RESET]			= QPHY_V5_PCS_SW_RESET,
+	[QPHY_START_CTRL]		= QPHY_V5_PCS_START_CONTROL,
+	[QPHY_PCS_STATUS]		= QPHY_V5_PCS_PCS_STATUS1,
+	[QPHY_PCS_POWER_DOWN_CONTROL]	= QPHY_V5_PCS_POWER_DOWN_CONTROL,
+	[QPHY_PCS_ENDPOINT_REFCLK_CNTRL]	= QPHY_PCIE_V6_20_PCS_ENDPOINT_REFCLK_CNTRL,
 };
 
 static const struct qmp_phy_init_tbl msm8998_pcie_serdes_tbl[] = {
@@ -2956,7 +2976,7 @@ static const struct qmp_phy_cfg sm8450_qmp_gen4x2_pciephy_cfg = {
 	.num_resets		= ARRAY_SIZE(sdm845_pciephy_reset_l),
 	.vreg_list		= qmp_phy_vreg_l,
 	.num_vregs		= ARRAY_SIZE(qmp_phy_vreg_l),
-	.regs			= pciephy_v5_regs_layout,
+	.regs			= pciephy_v5_20_regs_layout,
 
 	.pwrdn_ctrl		= SW_PWRDN | REFCLK_DRV_DSBL,
 	.phy_status		= PHYSTATUS_4_20,
@@ -3012,7 +3032,7 @@ static const struct qmp_phy_cfg sm8550_qmp_gen4x2_pciephy_cfg = {
 	.num_resets		= ARRAY_SIZE(sdm845_pciephy_reset_l),
 	.vreg_list		= sm8550_qmp_phy_vreg_l,
 	.num_vregs		= ARRAY_SIZE(sm8550_qmp_phy_vreg_l),
-	.regs			= pciephy_v5_regs_layout,
+	.regs			= pciephy_v6_20_regs_layout,
 
 	.pwrdn_ctrl		= SW_PWRDN | REFCLK_DRV_DSBL,
 	.phy_status		= PHYSTATUS_4_20,
@@ -3047,7 +3067,7 @@ static const struct qmp_phy_cfg sa8775p_qmp_gen4x2_pciephy_cfg = {
 	.num_resets		= ARRAY_SIZE(sdm845_pciephy_reset_l),
 	.vreg_list		= qmp_phy_vreg_l,
 	.num_vregs		= ARRAY_SIZE(qmp_phy_vreg_l),
-	.regs			= pciephy_v5_regs_layout,
+	.regs			= pciephy_v5_20_regs_layout,
 
 	.pwrdn_ctrl		= SW_PWRDN | REFCLK_DRV_DSBL,
 	.phy_status		= PHYSTATUS_4_20,
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h
index a469ae2a10a1..9b166286afda 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h
@@ -11,6 +11,7 @@
 #define QPHY_V5_PCS_PCIE_POWER_STATE_CONFIG2		0x0c
 #define QPHY_V5_PCS_PCIE_POWER_STATE_CONFIG4		0x14
 #define QPHY_V5_PCS_PCIE_ENDPOINT_REFCLK_DRIVE		0x20
+#define QPHY_V5_PCS_PCIE_ENDPOINT_REFCLK_CNTRL		0x24
 #define QPHY_V5_PCS_PCIE_INT_AUX_CLK_CONFIG1		0x54
 #define QPHY_V5_PCS_PCIE_OSC_DTCT_ACTIONS		0x94
 #define QPHY_V5_PCS_PCIE_EQ_CONFIG2			0xa8
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h
index cdf8c04ea078..8b114e538a07 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h
@@ -9,6 +9,7 @@
 /* Only for QMP V5_20 PHY - PCIe PCS registers */
 #define QPHY_V5_20_PCS_PCIE_POWER_STATE_CONFIG2		0x00c
 #define QPHY_V5_20_PCS_PCIE_ENDPOINT_REFCLK_DRIVE	0x01c
+#define QPHY_V5_20_PCS_PCIE_ENDPOINT_REFCLK_CNTRL	0x020
 #define QPHY_V5_20_PCS_PCIE_OSC_DTCT_MODE2_CONFIG5	0x084
 #define QPHY_V5_20_PCS_PCIE_OSC_DTCT_ACTIONS		0x090
 #define QPHY_V5_20_PCS_PCIE_EQ_CONFIG1			0x0a0
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h
index e3eb08776339..f7abe95c49ad 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h
@@ -10,6 +10,7 @@
 #define QPHY_PCIE_V6_20_PCS_POWER_STATE_CONFIG2		0x00c
 #define QPHY_PCIE_V6_20_PCS_TX_RX_CONFIG		0x018
 #define QPHY_PCIE_V6_20_PCS_ENDPOINT_REFCLK_DRIVE	0x01c
+#define QPHY_PCIE_V6_20_PCS_ENDPOINT_REFCLK_CNTRL	0x020
 #define QPHY_PCIE_V6_20_PCS_OSC_DTCT_ATCIONS		0x090
 #define QPHY_PCIE_V6_20_PCS_EQ_CONFIG1			0x0a0
 #define QPHY_PCIE_V6_20_PCS_EQ_CONFIG5			0x108

-- 
2.42.0

[PATCH v2 3/3] phy: qcom-qmp-pcie: Add support for keeping refclk always on

[Krishna chaitanya chundru]

In PCIe low power states like L1.1 or L1.2 the phy will stop
supplying refclk to endpoint. If endpoint asserts clkreq to bring
back link L0, then RC needs to provide refclk to endpoint.

If there is some issues in platform with clkreq signal propagation
to host and due to that host will not send refclk which results PCIe link
down. For those platforms phy needs to provide refclk even in low power
states.

Add a flag to keep refclk always supplied to endpoint.

Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
index 74d03d217ff2..a8d6d69e3f74 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
@@ -43,6 +43,8 @@
 /* QPHY_PCS_STATUS bit */
 #define PHYSTATUS				BIT(6)
 #define PHYSTATUS_4_20				BIT(7)
+/* PCS_PCIE_ENDPOINT_REFCLK_CNTRL */
+#define EPCLK_ALWAYS_ON_EN			BIT(6)
 
 #define PHY_INIT_COMPLETE_TIMEOUT		10000
 
@@ -2264,6 +2266,8 @@ struct qmp_pcie {
 	struct phy *phy;
 	int mode;
 
+	bool refclk_always_on;
+
 	struct clk_fixed_rate pipe_clk_fixed;
 };
 
@@ -3179,6 +3183,10 @@ static void qmp_pcie_init_registers(struct qmp_pcie *qmp, const struct qmp_phy_c
 	qmp_pcie_configure(pcs, tbls->pcs, tbls->pcs_num);
 	qmp_pcie_configure(pcs_misc, tbls->pcs_misc, tbls->pcs_misc_num);
 
+	if (qmp->refclk_always_on && cfg->regs[QPHY_PCS_ENDPOINT_REFCLK_CNTRL])
+		qphy_setbits(pcs_misc, cfg->regs[QPHY_PCS_ENDPOINT_REFCLK_CNTRL],
+			     EPCLK_ALWAYS_ON_EN);
+
 	if (cfg->lanes >= 4 && qmp->tcsr_4ln_config) {
 		qmp_pcie_configure(serdes, cfg->serdes_4ln_tbl, cfg->serdes_4ln_num);
 		qmp_pcie_init_port_b(qmp, tbls);
@@ -3701,6 +3709,12 @@ static int qmp_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_node_put;
 
+	qmp->refclk_always_on = of_property_read_bool(dev->of_node, "qcom,refclk-always-on");
+	if (qmp->refclk_always_on && !qmp->cfg->regs[QPHY_PCS_ENDPOINT_REFCLK_CNTRL]) {
+		dev_err(dev, "refclk is always on is present but refclk cntrl offset is not present\n");
+		goto err_node_put;
+	}
+
 	ret = phy_pipe_clk_register(qmp, np);
 	if (ret)
 		goto err_node_put;

-- 
2.42.0

Re: [PATCH v2 1/3] dt-bindings: phy: qcom,qmp: Add PCIe qcom,refclk-always-on property

[Dmitry Baryshkov]

On Tue, 7 Nov 2023 at 14:26, Krishna chaitanya chundru
<quic_krichai@quicinc.com> wrote:
>
> Document qcom,refclk-always-on property which is needed in some platforms
> to supply refclk even in PCIe low power states.
>
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  .../devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml        | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
> index 2c3d6553a7ba..263291447a5b 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
> @@ -93,6 +93,13 @@ properties:
>    "#phy-cells":
>      const: 0
>
> +  qcom,refclk-always-on:
> +    type: boolean
> +    description: If there is some issues in platform with clkreq signal

nit: there are some

However this still doesn't describe what kind of issues with clkreq
you observe. I mean, clkreq is just a GPIO pin.

> +      propagation to the host and due to that host will not send refclk, which
> +      results in linkdown in L1.2 or L1.1 exit initiated by EP. This property
> +      if set keeps refclk always on even in Low power states (optional).
> +
>  required:
>    - compatible
>    - reg
>
> --
> 2.42.0
>
>


-- 
With best wishes
Dmitry

Re: [PATCH v2 2/3] phy: qcom-qmp-pcie: Add endpoint refclk control register offset

[Dmitry Baryshkov]

On Tue, 7 Nov 2023 at 14:26, Krishna chaitanya chundru
<quic_krichai@quicinc.com> wrote:
>
> Some platforms needs to keep endpoint refclk always on, for this
> purpose add this offset for all the applicable phy versions.
>
> And also add reg layout for few controllers as we are adding
> endpoint refclk control register which changes based upon phy version.
>
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-pcie.c           | 26 +++++++++++++++++++---
>  drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h    |  1 +
>  drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h |  1 +
>  drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h |  1 +
>  4 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> index a63ca7424974..74d03d217ff2 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> @@ -77,6 +77,7 @@ enum qphy_reg_layout {
>         QPHY_START_CTRL,
>         QPHY_PCS_STATUS,
>         QPHY_PCS_POWER_DOWN_CONTROL,
> +       QPHY_PCS_ENDPOINT_REFCLK_CNTRL,
>         /* Keep last to ensure regs_layout arrays are properly initialized */
>         QPHY_LAYOUT_SIZE
>  };
> @@ -93,6 +94,7 @@ static const unsigned int pciephy_v3_regs_layout[QPHY_LAYOUT_SIZE] = {
>         [QPHY_START_CTRL]               = QPHY_V3_PCS_START_CONTROL,
>         [QPHY_PCS_STATUS]               = QPHY_V3_PCS_PCS_STATUS,
>         [QPHY_PCS_POWER_DOWN_CONTROL]   = QPHY_V3_PCS_POWER_DOWN_CONTROL,
> +       [QPHY_PCS_ENDPOINT_REFCLK_CNTRL]        = QPHY_V3_PCS_ENDPOINT_REFCLK_CNTRL,
>  };
>
>  static const unsigned int sdm845_qhp_pciephy_regs_layout[QPHY_LAYOUT_SIZE] = {
> @@ -107,6 +109,7 @@ static const unsigned int pciephy_v4_regs_layout[QPHY_LAYOUT_SIZE] = {
>         [QPHY_START_CTRL]               = QPHY_V4_PCS_START_CONTROL,
>         [QPHY_PCS_STATUS]               = QPHY_V4_PCS_PCS_STATUS1,
>         [QPHY_PCS_POWER_DOWN_CONTROL]   = QPHY_V4_PCS_POWER_DOWN_CONTROL,
> +       [QPHY_PCS_ENDPOINT_REFCLK_CNTRL]        = QPHY_V4_PCS_PCIE_ENDPOINT_REFCLK_CNTRL,
>  };
>
>  static const unsigned int pciephy_v5_regs_layout[QPHY_LAYOUT_SIZE] = {
> @@ -114,6 +117,23 @@ static const unsigned int pciephy_v5_regs_layout[QPHY_LAYOUT_SIZE] = {
>         [QPHY_START_CTRL]               = QPHY_V5_PCS_START_CONTROL,
>         [QPHY_PCS_STATUS]               = QPHY_V5_PCS_PCS_STATUS1,
>         [QPHY_PCS_POWER_DOWN_CONTROL]   = QPHY_V5_PCS_POWER_DOWN_CONTROL,
> +       [QPHY_PCS_ENDPOINT_REFCLK_CNTRL]        = QPHY_V5_PCS_PCIE_ENDPOINT_REFCLK_CNTRL,
> +};
> +
> +static const unsigned int pciephy_v5_20_regs_layout[QPHY_LAYOUT_SIZE] = {
> +       [QPHY_SW_RESET]                 = QPHY_V5_PCS_SW_RESET,
> +       [QPHY_START_CTRL]               = QPHY_V5_PCS_START_CONTROL,
> +       [QPHY_PCS_STATUS]               = QPHY_V5_PCS_PCS_STATUS1,
> +       [QPHY_PCS_POWER_DOWN_CONTROL]   = QPHY_V5_PCS_POWER_DOWN_CONTROL,

Nit: we should probably define V5_20 and v6_20 versions of these registers

If you were to send v3 for any reason, could you please add them?

> +       [QPHY_PCS_ENDPOINT_REFCLK_CNTRL]        = QPHY_V5_20_PCS_PCIE_ENDPOINT_REFCLK_CNTRL,
> +};
> +
> +static const unsigned int pciephy_v6_20_regs_layout[QPHY_LAYOUT_SIZE] = {
> +       [QPHY_SW_RESET]                 = QPHY_V5_PCS_SW_RESET,
> +       [QPHY_START_CTRL]               = QPHY_V5_PCS_START_CONTROL,
> +       [QPHY_PCS_STATUS]               = QPHY_V5_PCS_PCS_STATUS1,
> +       [QPHY_PCS_POWER_DOWN_CONTROL]   = QPHY_V5_PCS_POWER_DOWN_CONTROL,
> +       [QPHY_PCS_ENDPOINT_REFCLK_CNTRL]        = QPHY_PCIE_V6_20_PCS_ENDPOINT_REFCLK_CNTRL,
>  };
>
>  static const struct qmp_phy_init_tbl msm8998_pcie_serdes_tbl[] = {
> @@ -2956,7 +2976,7 @@ static const struct qmp_phy_cfg sm8450_qmp_gen4x2_pciephy_cfg = {
>         .num_resets             = ARRAY_SIZE(sdm845_pciephy_reset_l),
>         .vreg_list              = qmp_phy_vreg_l,
>         .num_vregs              = ARRAY_SIZE(qmp_phy_vreg_l),
> -       .regs                   = pciephy_v5_regs_layout,
> +       .regs                   = pciephy_v5_20_regs_layout,
>
>         .pwrdn_ctrl             = SW_PWRDN | REFCLK_DRV_DSBL,
>         .phy_status             = PHYSTATUS_4_20,
> @@ -3012,7 +3032,7 @@ static const struct qmp_phy_cfg sm8550_qmp_gen4x2_pciephy_cfg = {
>         .num_resets             = ARRAY_SIZE(sdm845_pciephy_reset_l),
>         .vreg_list              = sm8550_qmp_phy_vreg_l,
>         .num_vregs              = ARRAY_SIZE(sm8550_qmp_phy_vreg_l),
> -       .regs                   = pciephy_v5_regs_layout,
> +       .regs                   = pciephy_v6_20_regs_layout,
>
>         .pwrdn_ctrl             = SW_PWRDN | REFCLK_DRV_DSBL,
>         .phy_status             = PHYSTATUS_4_20,
> @@ -3047,7 +3067,7 @@ static const struct qmp_phy_cfg sa8775p_qmp_gen4x2_pciephy_cfg = {
>         .num_resets             = ARRAY_SIZE(sdm845_pciephy_reset_l),
>         .vreg_list              = qmp_phy_vreg_l,
>         .num_vregs              = ARRAY_SIZE(qmp_phy_vreg_l),
> -       .regs                   = pciephy_v5_regs_layout,
> +       .regs                   = pciephy_v5_20_regs_layout,
>
>         .pwrdn_ctrl             = SW_PWRDN | REFCLK_DRV_DSBL,
>         .phy_status             = PHYSTATUS_4_20,
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h
> index a469ae2a10a1..9b166286afda 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h
> @@ -11,6 +11,7 @@
>  #define QPHY_V5_PCS_PCIE_POWER_STATE_CONFIG2           0x0c
>  #define QPHY_V5_PCS_PCIE_POWER_STATE_CONFIG4           0x14
>  #define QPHY_V5_PCS_PCIE_ENDPOINT_REFCLK_DRIVE         0x20
> +#define QPHY_V5_PCS_PCIE_ENDPOINT_REFCLK_CNTRL         0x24
>  #define QPHY_V5_PCS_PCIE_INT_AUX_CLK_CONFIG1           0x54
>  #define QPHY_V5_PCS_PCIE_OSC_DTCT_ACTIONS              0x94
>  #define QPHY_V5_PCS_PCIE_EQ_CONFIG2                    0xa8
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h
> index cdf8c04ea078..8b114e538a07 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h
> @@ -9,6 +9,7 @@
>  /* Only for QMP V5_20 PHY - PCIe PCS registers */
>  #define QPHY_V5_20_PCS_PCIE_POWER_STATE_CONFIG2                0x00c
>  #define QPHY_V5_20_PCS_PCIE_ENDPOINT_REFCLK_DRIVE      0x01c
> +#define QPHY_V5_20_PCS_PCIE_ENDPOINT_REFCLK_CNTRL      0x020
>  #define QPHY_V5_20_PCS_PCIE_OSC_DTCT_MODE2_CONFIG5     0x084
>  #define QPHY_V5_20_PCS_PCIE_OSC_DTCT_ACTIONS           0x090
>  #define QPHY_V5_20_PCS_PCIE_EQ_CONFIG1                 0x0a0
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h
> index e3eb08776339..f7abe95c49ad 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h
> @@ -10,6 +10,7 @@
>  #define QPHY_PCIE_V6_20_PCS_POWER_STATE_CONFIG2                0x00c
>  #define QPHY_PCIE_V6_20_PCS_TX_RX_CONFIG               0x018
>  #define QPHY_PCIE_V6_20_PCS_ENDPOINT_REFCLK_DRIVE      0x01c
> +#define QPHY_PCIE_V6_20_PCS_ENDPOINT_REFCLK_CNTRL      0x020
>  #define QPHY_PCIE_V6_20_PCS_OSC_DTCT_ATCIONS           0x090
>  #define QPHY_PCIE_V6_20_PCS_EQ_CONFIG1                 0x0a0
>  #define QPHY_PCIE_V6_20_PCS_EQ_CONFIG5                 0x108
>
> --
> 2.42.0
>
>


-- 
With best wishes
Dmitry

Re: [PATCH v2 3/3] phy: qcom-qmp-pcie: Add support for keeping refclk always on

[Dmitry Baryshkov]

On Tue, 7 Nov 2023 at 14:26, Krishna chaitanya chundru
<quic_krichai@quicinc.com> wrote:
>
> In PCIe low power states like L1.1 or L1.2 the phy will stop
> supplying refclk to endpoint. If endpoint asserts clkreq to bring
> back link L0, then RC needs to provide refclk to endpoint.
>
> If there is some issues in platform with clkreq signal propagation

Nit: there are

I'd rephrase this in the following way:

Some platforms (e.g. ABC DEF) fail to drive the clkreq signal to the
host (because it is unconnected ?). Due to that...

Other than that:

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> to host and due to that host will not send refclk which results PCIe link
> down. For those platforms phy needs to provide refclk even in low power
> states.
>
> Add a flag to keep refclk always supplied to endpoint.
>
> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> index 74d03d217ff2..a8d6d69e3f74 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> @@ -43,6 +43,8 @@
>  /* QPHY_PCS_STATUS bit */
>  #define PHYSTATUS                              BIT(6)
>  #define PHYSTATUS_4_20                         BIT(7)
> +/* PCS_PCIE_ENDPOINT_REFCLK_CNTRL */
> +#define EPCLK_ALWAYS_ON_EN                     BIT(6)
>
>  #define PHY_INIT_COMPLETE_TIMEOUT              10000
>
> @@ -2264,6 +2266,8 @@ struct qmp_pcie {
>         struct phy *phy;
>         int mode;
>
> +       bool refclk_always_on;
> +
>         struct clk_fixed_rate pipe_clk_fixed;
>  };
>
> @@ -3179,6 +3183,10 @@ static void qmp_pcie_init_registers(struct qmp_pcie *qmp, const struct qmp_phy_c
>         qmp_pcie_configure(pcs, tbls->pcs, tbls->pcs_num);
>         qmp_pcie_configure(pcs_misc, tbls->pcs_misc, tbls->pcs_misc_num);
>
> +       if (qmp->refclk_always_on && cfg->regs[QPHY_PCS_ENDPOINT_REFCLK_CNTRL])
> +               qphy_setbits(pcs_misc, cfg->regs[QPHY_PCS_ENDPOINT_REFCLK_CNTRL],
> +                            EPCLK_ALWAYS_ON_EN);
> +
>         if (cfg->lanes >= 4 && qmp->tcsr_4ln_config) {
>                 qmp_pcie_configure(serdes, cfg->serdes_4ln_tbl, cfg->serdes_4ln_num);
>                 qmp_pcie_init_port_b(qmp, tbls);
> @@ -3701,6 +3709,12 @@ static int qmp_pcie_probe(struct platform_device *pdev)
>         if (ret)
>                 goto err_node_put;
>
> +       qmp->refclk_always_on = of_property_read_bool(dev->of_node, "qcom,refclk-always-on");
> +       if (qmp->refclk_always_on && !qmp->cfg->regs[QPHY_PCS_ENDPOINT_REFCLK_CNTRL]) {
> +               dev_err(dev, "refclk is always on is present but refclk cntrl offset is not present\n");
> +               goto err_node_put;
> +       }
> +
>         ret = phy_pipe_clk_register(qmp, np);
>         if (ret)
>                 goto err_node_put;
>
> --
> 2.42.0
>
>


-- 
With best wishes
Dmitry

Re: [PATCH v2 0/3] phy: qcom-qmp-pcie: Add support to keep refclk always on

[Konrad Dybcio]

On 11/7/23 13:26, Krishna chaitanya chundru wrote:
> This series adds support to provide refclk to endpoint even in low
> power states.
> 
> Due to some platform specific issues with CLKREQ signal, it is not being
> propagated to the host and as host doesn't know the clkreq signal host is
> not sending refclk. Due to this endpoint is seeing linkdown and going
> to bad state.
> To avoid those ref clk should be provided always to the endpoint. The
> issue is coming only when ep intiates the L1.1 or L1.2 exit and clkreq
> is not being propagated properly to the host.
I'm gonna sound like a broken record, but:

How much power does this consume? Would it matter if we kept this
clock always-on for all platforms with this version of the phy?

Konrad

Re: [PATCH v2 1/3] dt-bindings: phy: qcom,qmp: Add PCIe qcom,refclk-always-on property

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1868 bytes --]

On Tue, Nov 07, 2023 at 03:01:47PM +0200, Dmitry Baryshkov wrote:
> On Tue, 7 Nov 2023 at 14:26, Krishna chaitanya chundru
> <quic_krichai@quicinc.com> wrote:
> >
> > Document qcom,refclk-always-on property which is needed in some platforms
> > to supply refclk even in PCIe low power states.
> >
> > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> > ---
> >  .../devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml        | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
> > index 2c3d6553a7ba..263291447a5b 100644
> > --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
> > +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
> > @@ -93,6 +93,13 @@ properties:
> >    "#phy-cells":
> >      const: 0
> >
> > +  qcom,refclk-always-on:
> > +    type: boolean
> > +    description: If there is some issues in platform with clkreq signal
> 
> nit: there are some
> 
> However this still doesn't describe what kind of issues with clkreq
> you observe. I mean, clkreq is just a GPIO pin.
> 
> > +      propagation to the host and due to that host will not send refclk, which
> > +      results in linkdown in L1.2 or L1.1 exit initiated by EP. This property
> > +      if set keeps refclk always on even in Low power states (optional).

Dimitry's issues with the property aside, putting "(optional)" in the
description is meaningless - qcom,refclk-always-on isn't listed in the
required properties section, so therefore has to be optional.

Cheers,
Conor.

> > +
> >  required:
> >    - compatible
> >    - reg
> >
> > --
> > 2.42.0
> >
> >
> 
> 
> -- 
> With best wishes
> Dmitry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 0/3] phy: qcom-qmp-pcie: Add support to keep refclk always on

[Krishna Chaitanya Chundru]

On 11/8/2023 3:27 AM, Konrad Dybcio wrote:
>
>
> On 11/7/23 13:26, Krishna chaitanya chundru wrote:
>> This series adds support to provide refclk to endpoint even in low
>> power states.
>>
>> Due to some platform specific issues with CLKREQ signal, it is not being
>> propagated to the host and as host doesn't know the clkreq signal 
>> host is
>> not sending refclk. Due to this endpoint is seeing linkdown and going
>> to bad state.
>> To avoid those ref clk should be provided always to the endpoint. The
>> issue is coming only when ep intiates the L1.1 or L1.2 exit and clkreq
>> is not being propagated properly to the host.
> I'm gonna sound like a broken record, but:
>
> How much power does this consume? Would it matter if we kept this
> clock always-on for all platforms with this version of the phy?
>
> Konrad

We see about 22mw extra power consumption with refclk always on.

We can't keep this property always on as there is impact on power 
consumption.

- Krishna Chaitanya

Re: [PATCH v2 2/3] phy: qcom-qmp-pcie: Add endpoint refclk control register offset

[Krishna Chaitanya Chundru]

On 11/7/2023 6:36 PM, Dmitry Baryshkov wrote:
> On Tue, 7 Nov 2023 at 14:26, Krishna chaitanya chundru
> <quic_krichai@quicinc.com> wrote:
>> Some platforms needs to keep endpoint refclk always on, for this
>> purpose add this offset for all the applicable phy versions.
>>
>> And also add reg layout for few controllers as we are adding
>> endpoint refclk control register which changes based upon phy version.
>>
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>
>> ---
>>   drivers/phy/qualcomm/phy-qcom-qmp-pcie.c           | 26 +++++++++++++++++++---
>>   drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h    |  1 +
>>   drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h |  1 +
>>   drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h |  1 +
>>   4 files changed, 26 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
>> index a63ca7424974..74d03d217ff2 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
>> @@ -77,6 +77,7 @@ enum qphy_reg_layout {
>>          QPHY_START_CTRL,
>>          QPHY_PCS_STATUS,
>>          QPHY_PCS_POWER_DOWN_CONTROL,
>> +       QPHY_PCS_ENDPOINT_REFCLK_CNTRL,
>>          /* Keep last to ensure regs_layout arrays are properly initialized */
>>          QPHY_LAYOUT_SIZE
>>   };
>> @@ -93,6 +94,7 @@ static const unsigned int pciephy_v3_regs_layout[QPHY_LAYOUT_SIZE] = {
>>          [QPHY_START_CTRL]               = QPHY_V3_PCS_START_CONTROL,
>>          [QPHY_PCS_STATUS]               = QPHY_V3_PCS_PCS_STATUS,
>>          [QPHY_PCS_POWER_DOWN_CONTROL]   = QPHY_V3_PCS_POWER_DOWN_CONTROL,
>> +       [QPHY_PCS_ENDPOINT_REFCLK_CNTRL]        = QPHY_V3_PCS_ENDPOINT_REFCLK_CNTRL,
>>   };
>>
>>   static const unsigned int sdm845_qhp_pciephy_regs_layout[QPHY_LAYOUT_SIZE] = {
>> @@ -107,6 +109,7 @@ static const unsigned int pciephy_v4_regs_layout[QPHY_LAYOUT_SIZE] = {
>>          [QPHY_START_CTRL]               = QPHY_V4_PCS_START_CONTROL,
>>          [QPHY_PCS_STATUS]               = QPHY_V4_PCS_PCS_STATUS1,
>>          [QPHY_PCS_POWER_DOWN_CONTROL]   = QPHY_V4_PCS_POWER_DOWN_CONTROL,
>> +       [QPHY_PCS_ENDPOINT_REFCLK_CNTRL]        = QPHY_V4_PCS_PCIE_ENDPOINT_REFCLK_CNTRL,
>>   };
>>
>>   static const unsigned int pciephy_v5_regs_layout[QPHY_LAYOUT_SIZE] = {
>> @@ -114,6 +117,23 @@ static const unsigned int pciephy_v5_regs_layout[QPHY_LAYOUT_SIZE] = {
>>          [QPHY_START_CTRL]               = QPHY_V5_PCS_START_CONTROL,
>>          [QPHY_PCS_STATUS]               = QPHY_V5_PCS_PCS_STATUS1,
>>          [QPHY_PCS_POWER_DOWN_CONTROL]   = QPHY_V5_PCS_POWER_DOWN_CONTROL,
>> +       [QPHY_PCS_ENDPOINT_REFCLK_CNTRL]        = QPHY_V5_PCS_PCIE_ENDPOINT_REFCLK_CNTRL,
>> +};
>> +
>> +static const unsigned int pciephy_v5_20_regs_layout[QPHY_LAYOUT_SIZE] = {
>> +       [QPHY_SW_RESET]                 = QPHY_V5_PCS_SW_RESET,
>> +       [QPHY_START_CTRL]               = QPHY_V5_PCS_START_CONTROL,
>> +       [QPHY_PCS_STATUS]               = QPHY_V5_PCS_PCS_STATUS1,
>> +       [QPHY_PCS_POWER_DOWN_CONTROL]   = QPHY_V5_PCS_POWER_DOWN_CONTROL,
> Nit: we should probably define V5_20 and v6_20 versions of these registers

We don't have separate defines for v5_20 and v6_20 for these registers, 
that is why we are using these.

And the offsets are same for those version. That is why I tried to use 
macros.

- Krishna Chaitanya.

> If you were to send v3 for any reason, could you please add them?
>
>> +       [QPHY_PCS_ENDPOINT_REFCLK_CNTRL]        = QPHY_V5_20_PCS_PCIE_ENDPOINT_REFCLK_CNTRL,
>> +};
>> +
>> +static const unsigned int pciephy_v6_20_regs_layout[QPHY_LAYOUT_SIZE] = {
>> +       [QPHY_SW_RESET]                 = QPHY_V5_PCS_SW_RESET,
>> +       [QPHY_START_CTRL]               = QPHY_V5_PCS_START_CONTROL,
>> +       [QPHY_PCS_STATUS]               = QPHY_V5_PCS_PCS_STATUS1,
>> +       [QPHY_PCS_POWER_DOWN_CONTROL]   = QPHY_V5_PCS_POWER_DOWN_CONTROL,
>> +       [QPHY_PCS_ENDPOINT_REFCLK_CNTRL]        = QPHY_PCIE_V6_20_PCS_ENDPOINT_REFCLK_CNTRL,
>>   };
>>
>>   static const struct qmp_phy_init_tbl msm8998_pcie_serdes_tbl[] = {
>> @@ -2956,7 +2976,7 @@ static const struct qmp_phy_cfg sm8450_qmp_gen4x2_pciephy_cfg = {
>>          .num_resets             = ARRAY_SIZE(sdm845_pciephy_reset_l),
>>          .vreg_list              = qmp_phy_vreg_l,
>>          .num_vregs              = ARRAY_SIZE(qmp_phy_vreg_l),
>> -       .regs                   = pciephy_v5_regs_layout,
>> +       .regs                   = pciephy_v5_20_regs_layout,
>>
>>          .pwrdn_ctrl             = SW_PWRDN | REFCLK_DRV_DSBL,
>>          .phy_status             = PHYSTATUS_4_20,
>> @@ -3012,7 +3032,7 @@ static const struct qmp_phy_cfg sm8550_qmp_gen4x2_pciephy_cfg = {
>>          .num_resets             = ARRAY_SIZE(sdm845_pciephy_reset_l),
>>          .vreg_list              = sm8550_qmp_phy_vreg_l,
>>          .num_vregs              = ARRAY_SIZE(sm8550_qmp_phy_vreg_l),
>> -       .regs                   = pciephy_v5_regs_layout,
>> +       .regs                   = pciephy_v6_20_regs_layout,
>>
>>          .pwrdn_ctrl             = SW_PWRDN | REFCLK_DRV_DSBL,
>>          .phy_status             = PHYSTATUS_4_20,
>> @@ -3047,7 +3067,7 @@ static const struct qmp_phy_cfg sa8775p_qmp_gen4x2_pciephy_cfg = {
>>          .num_resets             = ARRAY_SIZE(sdm845_pciephy_reset_l),
>>          .vreg_list              = qmp_phy_vreg_l,
>>          .num_vregs              = ARRAY_SIZE(qmp_phy_vreg_l),
>> -       .regs                   = pciephy_v5_regs_layout,
>> +       .regs                   = pciephy_v5_20_regs_layout,
>>
>>          .pwrdn_ctrl             = SW_PWRDN | REFCLK_DRV_DSBL,
>>          .phy_status             = PHYSTATUS_4_20,
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h
>> index a469ae2a10a1..9b166286afda 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h
>> @@ -11,6 +11,7 @@
>>   #define QPHY_V5_PCS_PCIE_POWER_STATE_CONFIG2           0x0c
>>   #define QPHY_V5_PCS_PCIE_POWER_STATE_CONFIG4           0x14
>>   #define QPHY_V5_PCS_PCIE_ENDPOINT_REFCLK_DRIVE         0x20
>> +#define QPHY_V5_PCS_PCIE_ENDPOINT_REFCLK_CNTRL         0x24
>>   #define QPHY_V5_PCS_PCIE_INT_AUX_CLK_CONFIG1           0x54
>>   #define QPHY_V5_PCS_PCIE_OSC_DTCT_ACTIONS              0x94
>>   #define QPHY_V5_PCS_PCIE_EQ_CONFIG2                    0xa8
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h
>> index cdf8c04ea078..8b114e538a07 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h
>> @@ -9,6 +9,7 @@
>>   /* Only for QMP V5_20 PHY - PCIe PCS registers */
>>   #define QPHY_V5_20_PCS_PCIE_POWER_STATE_CONFIG2                0x00c
>>   #define QPHY_V5_20_PCS_PCIE_ENDPOINT_REFCLK_DRIVE      0x01c
>> +#define QPHY_V5_20_PCS_PCIE_ENDPOINT_REFCLK_CNTRL      0x020
>>   #define QPHY_V5_20_PCS_PCIE_OSC_DTCT_MODE2_CONFIG5     0x084
>>   #define QPHY_V5_20_PCS_PCIE_OSC_DTCT_ACTIONS           0x090
>>   #define QPHY_V5_20_PCS_PCIE_EQ_CONFIG1                 0x0a0
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h
>> index e3eb08776339..f7abe95c49ad 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h
>> @@ -10,6 +10,7 @@
>>   #define QPHY_PCIE_V6_20_PCS_POWER_STATE_CONFIG2                0x00c
>>   #define QPHY_PCIE_V6_20_PCS_TX_RX_CONFIG               0x018
>>   #define QPHY_PCIE_V6_20_PCS_ENDPOINT_REFCLK_DRIVE      0x01c
>> +#define QPHY_PCIE_V6_20_PCS_ENDPOINT_REFCLK_CNTRL      0x020
>>   #define QPHY_PCIE_V6_20_PCS_OSC_DTCT_ATCIONS           0x090
>>   #define QPHY_PCIE_V6_20_PCS_EQ_CONFIG1                 0x0a0
>>   #define QPHY_PCIE_V6_20_PCS_EQ_CONFIG5                 0x108
>>
>> --
>> 2.42.0
>>
>>
>

Re: [PATCH v2 2/3] phy: qcom-qmp-pcie: Add endpoint refclk control register offset

[Dmitry Baryshkov]

On Thu, 9 Nov 2023 at 11:36, Krishna Chaitanya Chundru
<quic_krichai@quicinc.com> wrote:
>
>
> On 11/7/2023 6:36 PM, Dmitry Baryshkov wrote:
> > On Tue, 7 Nov 2023 at 14:26, Krishna chaitanya chundru
> > <quic_krichai@quicinc.com> wrote:
> >> Some platforms needs to keep endpoint refclk always on, for this
> >> purpose add this offset for all the applicable phy versions.
> >>
> >> And also add reg layout for few controllers as we are adding
> >> endpoint refclk control register which changes based upon phy version.
> >>
> >> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >
> >> ---
> >>   drivers/phy/qualcomm/phy-qcom-qmp-pcie.c           | 26 +++++++++++++++++++---
> >>   drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h    |  1 +
> >>   drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h |  1 +
> >>   drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h |  1 +
> >>   4 files changed, 26 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> >> index a63ca7424974..74d03d217ff2 100644
> >> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> >> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> >> @@ -77,6 +77,7 @@ enum qphy_reg_layout {
> >>          QPHY_START_CTRL,
> >>          QPHY_PCS_STATUS,
> >>          QPHY_PCS_POWER_DOWN_CONTROL,
> >> +       QPHY_PCS_ENDPOINT_REFCLK_CNTRL,
> >>          /* Keep last to ensure regs_layout arrays are properly initialized */
> >>          QPHY_LAYOUT_SIZE
> >>   };
> >> @@ -93,6 +94,7 @@ static const unsigned int pciephy_v3_regs_layout[QPHY_LAYOUT_SIZE] = {
> >>          [QPHY_START_CTRL]               = QPHY_V3_PCS_START_CONTROL,
> >>          [QPHY_PCS_STATUS]               = QPHY_V3_PCS_PCS_STATUS,
> >>          [QPHY_PCS_POWER_DOWN_CONTROL]   = QPHY_V3_PCS_POWER_DOWN_CONTROL,
> >> +       [QPHY_PCS_ENDPOINT_REFCLK_CNTRL]        = QPHY_V3_PCS_ENDPOINT_REFCLK_CNTRL,
> >>   };
> >>
> >>   static const unsigned int sdm845_qhp_pciephy_regs_layout[QPHY_LAYOUT_SIZE] = {
> >> @@ -107,6 +109,7 @@ static const unsigned int pciephy_v4_regs_layout[QPHY_LAYOUT_SIZE] = {
> >>          [QPHY_START_CTRL]               = QPHY_V4_PCS_START_CONTROL,
> >>          [QPHY_PCS_STATUS]               = QPHY_V4_PCS_PCS_STATUS1,
> >>          [QPHY_PCS_POWER_DOWN_CONTROL]   = QPHY_V4_PCS_POWER_DOWN_CONTROL,
> >> +       [QPHY_PCS_ENDPOINT_REFCLK_CNTRL]        = QPHY_V4_PCS_PCIE_ENDPOINT_REFCLK_CNTRL,
> >>   };
> >>
> >>   static const unsigned int pciephy_v5_regs_layout[QPHY_LAYOUT_SIZE] = {
> >> @@ -114,6 +117,23 @@ static const unsigned int pciephy_v5_regs_layout[QPHY_LAYOUT_SIZE] = {
> >>          [QPHY_START_CTRL]               = QPHY_V5_PCS_START_CONTROL,
> >>          [QPHY_PCS_STATUS]               = QPHY_V5_PCS_PCS_STATUS1,
> >>          [QPHY_PCS_POWER_DOWN_CONTROL]   = QPHY_V5_PCS_POWER_DOWN_CONTROL,
> >> +       [QPHY_PCS_ENDPOINT_REFCLK_CNTRL]        = QPHY_V5_PCS_PCIE_ENDPOINT_REFCLK_CNTRL,
> >> +};
> >> +
> >> +static const unsigned int pciephy_v5_20_regs_layout[QPHY_LAYOUT_SIZE] = {
> >> +       [QPHY_SW_RESET]                 = QPHY_V5_PCS_SW_RESET,
> >> +       [QPHY_START_CTRL]               = QPHY_V5_PCS_START_CONTROL,
> >> +       [QPHY_PCS_STATUS]               = QPHY_V5_PCS_PCS_STATUS1,
> >> +       [QPHY_PCS_POWER_DOWN_CONTROL]   = QPHY_V5_PCS_POWER_DOWN_CONTROL,
> > Nit: we should probably define V5_20 and v6_20 versions of these registers
>
> We don't have separate defines for v5_20 and v6_20 for these registers,
> that is why we are using these.
>
> And the offsets are same for those version. That is why I tried to use
> macros.

Mixing versions in a single table can quickly lead to a disaster.

>
> - Krishna Chaitanya.
>
> > If you were to send v3 for any reason, could you please add them?
> >
> >> +       [QPHY_PCS_ENDPOINT_REFCLK_CNTRL]        = QPHY_V5_20_PCS_PCIE_ENDPOINT_REFCLK_CNTRL,
> >> +};
> >> +
> >> +static const unsigned int pciephy_v6_20_regs_layout[QPHY_LAYOUT_SIZE] = {
> >> +       [QPHY_SW_RESET]                 = QPHY_V5_PCS_SW_RESET,
> >> +       [QPHY_START_CTRL]               = QPHY_V5_PCS_START_CONTROL,
> >> +       [QPHY_PCS_STATUS]               = QPHY_V5_PCS_PCS_STATUS1,
> >> +       [QPHY_PCS_POWER_DOWN_CONTROL]   = QPHY_V5_PCS_POWER_DOWN_CONTROL,
> >> +       [QPHY_PCS_ENDPOINT_REFCLK_CNTRL]        = QPHY_PCIE_V6_20_PCS_ENDPOINT_REFCLK_CNTRL,
> >>   };
> >>
> >>   static const struct qmp_phy_init_tbl msm8998_pcie_serdes_tbl[] = {
> >> @@ -2956,7 +2976,7 @@ static const struct qmp_phy_cfg sm8450_qmp_gen4x2_pciephy_cfg = {
> >>          .num_resets             = ARRAY_SIZE(sdm845_pciephy_reset_l),
> >>          .vreg_list              = qmp_phy_vreg_l,
> >>          .num_vregs              = ARRAY_SIZE(qmp_phy_vreg_l),
> >> -       .regs                   = pciephy_v5_regs_layout,
> >> +       .regs                   = pciephy_v5_20_regs_layout,
> >>
> >>          .pwrdn_ctrl             = SW_PWRDN | REFCLK_DRV_DSBL,
> >>          .phy_status             = PHYSTATUS_4_20,
> >> @@ -3012,7 +3032,7 @@ static const struct qmp_phy_cfg sm8550_qmp_gen4x2_pciephy_cfg = {
> >>          .num_resets             = ARRAY_SIZE(sdm845_pciephy_reset_l),
> >>          .vreg_list              = sm8550_qmp_phy_vreg_l,
> >>          .num_vregs              = ARRAY_SIZE(sm8550_qmp_phy_vreg_l),
> >> -       .regs                   = pciephy_v5_regs_layout,
> >> +       .regs                   = pciephy_v6_20_regs_layout,
> >>
> >>          .pwrdn_ctrl             = SW_PWRDN | REFCLK_DRV_DSBL,
> >>          .phy_status             = PHYSTATUS_4_20,
> >> @@ -3047,7 +3067,7 @@ static const struct qmp_phy_cfg sa8775p_qmp_gen4x2_pciephy_cfg = {
> >>          .num_resets             = ARRAY_SIZE(sdm845_pciephy_reset_l),
> >>          .vreg_list              = qmp_phy_vreg_l,
> >>          .num_vregs              = ARRAY_SIZE(qmp_phy_vreg_l),
> >> -       .regs                   = pciephy_v5_regs_layout,
> >> +       .regs                   = pciephy_v5_20_regs_layout,
> >>
> >>          .pwrdn_ctrl             = SW_PWRDN | REFCLK_DRV_DSBL,
> >>          .phy_status             = PHYSTATUS_4_20,
> >> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h
> >> index a469ae2a10a1..9b166286afda 100644
> >> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h
> >> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h
> >> @@ -11,6 +11,7 @@
> >>   #define QPHY_V5_PCS_PCIE_POWER_STATE_CONFIG2           0x0c
> >>   #define QPHY_V5_PCS_PCIE_POWER_STATE_CONFIG4           0x14
> >>   #define QPHY_V5_PCS_PCIE_ENDPOINT_REFCLK_DRIVE         0x20
> >> +#define QPHY_V5_PCS_PCIE_ENDPOINT_REFCLK_CNTRL         0x24
> >>   #define QPHY_V5_PCS_PCIE_INT_AUX_CLK_CONFIG1           0x54
> >>   #define QPHY_V5_PCS_PCIE_OSC_DTCT_ACTIONS              0x94
> >>   #define QPHY_V5_PCS_PCIE_EQ_CONFIG2                    0xa8
> >> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h
> >> index cdf8c04ea078..8b114e538a07 100644
> >> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h
> >> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h
> >> @@ -9,6 +9,7 @@
> >>   /* Only for QMP V5_20 PHY - PCIe PCS registers */
> >>   #define QPHY_V5_20_PCS_PCIE_POWER_STATE_CONFIG2                0x00c
> >>   #define QPHY_V5_20_PCS_PCIE_ENDPOINT_REFCLK_DRIVE      0x01c
> >> +#define QPHY_V5_20_PCS_PCIE_ENDPOINT_REFCLK_CNTRL      0x020
> >>   #define QPHY_V5_20_PCS_PCIE_OSC_DTCT_MODE2_CONFIG5     0x084
> >>   #define QPHY_V5_20_PCS_PCIE_OSC_DTCT_ACTIONS           0x090
> >>   #define QPHY_V5_20_PCS_PCIE_EQ_CONFIG1                 0x0a0
> >> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h
> >> index e3eb08776339..f7abe95c49ad 100644
> >> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h
> >> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h
> >> @@ -10,6 +10,7 @@
> >>   #define QPHY_PCIE_V6_20_PCS_POWER_STATE_CONFIG2                0x00c
> >>   #define QPHY_PCIE_V6_20_PCS_TX_RX_CONFIG               0x018
> >>   #define QPHY_PCIE_V6_20_PCS_ENDPOINT_REFCLK_DRIVE      0x01c
> >> +#define QPHY_PCIE_V6_20_PCS_ENDPOINT_REFCLK_CNTRL      0x020
> >>   #define QPHY_PCIE_V6_20_PCS_OSC_DTCT_ATCIONS           0x090
> >>   #define QPHY_PCIE_V6_20_PCS_EQ_CONFIG1                 0x0a0
> >>   #define QPHY_PCIE_V6_20_PCS_EQ_CONFIG5                 0x108
> >>
> >> --
> >> 2.42.0
> >>
> >>
> >



-- 
With best wishes
Dmitry

Re: [PATCH v2 2/3] phy: qcom-qmp-pcie: Add endpoint refclk control register offset

[Krishna Chaitanya Chundru]

On 11/9/2023 3:15 PM, Dmitry Baryshkov wrote:
> On Thu, 9 Nov 2023 at 11:36, Krishna Chaitanya Chundru
> <quic_krichai@quicinc.com> wrote:
>>
>> On 11/7/2023 6:36 PM, Dmitry Baryshkov wrote:
>>> On Tue, 7 Nov 2023 at 14:26, Krishna chaitanya chundru
>>> <quic_krichai@quicinc.com> wrote:
>>>> Some platforms needs to keep endpoint refclk always on, for this
>>>> purpose add this offset for all the applicable phy versions.
>>>>
>>>> And also add reg layout for few controllers as we are adding
>>>> endpoint refclk control register which changes based upon phy version.
>>>>
>>>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>
>>>> ---
>>>>    drivers/phy/qualcomm/phy-qcom-qmp-pcie.c           | 26 +++++++++++++++++++---
>>>>    drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h    |  1 +
>>>>    drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h |  1 +
>>>>    drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h |  1 +
>>>>    4 files changed, 26 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
>>>> index a63ca7424974..74d03d217ff2 100644
>>>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
>>>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
>>>> @@ -77,6 +77,7 @@ enum qphy_reg_layout {
>>>>           QPHY_START_CTRL,
>>>>           QPHY_PCS_STATUS,
>>>>           QPHY_PCS_POWER_DOWN_CONTROL,
>>>> +       QPHY_PCS_ENDPOINT_REFCLK_CNTRL,
>>>>           /* Keep last to ensure regs_layout arrays are properly initialized */
>>>>           QPHY_LAYOUT_SIZE
>>>>    };
>>>> @@ -93,6 +94,7 @@ static const unsigned int pciephy_v3_regs_layout[QPHY_LAYOUT_SIZE] = {
>>>>           [QPHY_START_CTRL]               = QPHY_V3_PCS_START_CONTROL,
>>>>           [QPHY_PCS_STATUS]               = QPHY_V3_PCS_PCS_STATUS,
>>>>           [QPHY_PCS_POWER_DOWN_CONTROL]   = QPHY_V3_PCS_POWER_DOWN_CONTROL,
>>>> +       [QPHY_PCS_ENDPOINT_REFCLK_CNTRL]        = QPHY_V3_PCS_ENDPOINT_REFCLK_CNTRL,
>>>>    };
>>>>
>>>>    static const unsigned int sdm845_qhp_pciephy_regs_layout[QPHY_LAYOUT_SIZE] = {
>>>> @@ -107,6 +109,7 @@ static const unsigned int pciephy_v4_regs_layout[QPHY_LAYOUT_SIZE] = {
>>>>           [QPHY_START_CTRL]               = QPHY_V4_PCS_START_CONTROL,
>>>>           [QPHY_PCS_STATUS]               = QPHY_V4_PCS_PCS_STATUS1,
>>>>           [QPHY_PCS_POWER_DOWN_CONTROL]   = QPHY_V4_PCS_POWER_DOWN_CONTROL,
>>>> +       [QPHY_PCS_ENDPOINT_REFCLK_CNTRL]        = QPHY_V4_PCS_PCIE_ENDPOINT_REFCLK_CNTRL,
>>>>    };
>>>>
>>>>    static const unsigned int pciephy_v5_regs_layout[QPHY_LAYOUT_SIZE] = {
>>>> @@ -114,6 +117,23 @@ static const unsigned int pciephy_v5_regs_layout[QPHY_LAYOUT_SIZE] = {
>>>>           [QPHY_START_CTRL]               = QPHY_V5_PCS_START_CONTROL,
>>>>           [QPHY_PCS_STATUS]               = QPHY_V5_PCS_PCS_STATUS1,
>>>>           [QPHY_PCS_POWER_DOWN_CONTROL]   = QPHY_V5_PCS_POWER_DOWN_CONTROL,
>>>> +       [QPHY_PCS_ENDPOINT_REFCLK_CNTRL]        = QPHY_V5_PCS_PCIE_ENDPOINT_REFCLK_CNTRL,
>>>> +};
>>>> +
>>>> +static const unsigned int pciephy_v5_20_regs_layout[QPHY_LAYOUT_SIZE] = {
>>>> +       [QPHY_SW_RESET]                 = QPHY_V5_PCS_SW_RESET,
>>>> +       [QPHY_START_CTRL]               = QPHY_V5_PCS_START_CONTROL,
>>>> +       [QPHY_PCS_STATUS]               = QPHY_V5_PCS_PCS_STATUS1,
>>>> +       [QPHY_PCS_POWER_DOWN_CONTROL]   = QPHY_V5_PCS_POWER_DOWN_CONTROL,
>>> Nit: we should probably define V5_20 and v6_20 versions of these registers
>> We don't have separate defines for v5_20 and v6_20 for these registers,
>> that is why we are using these.
>>
>> And the offsets are same for those version. That is why I tried to use
>> macros.
> Mixing versions in a single table can quickly lead to a disaster.

I will add then in the next series.

- Krishna Chaitanya

>> - Krishna Chaitanya.
>>
>>> If you were to send v3 for any reason, could you please add them?
>>>
>>>> +       [QPHY_PCS_ENDPOINT_REFCLK_CNTRL]        = QPHY_V5_20_PCS_PCIE_ENDPOINT_REFCLK_CNTRL,
>>>> +};
>>>> +
>>>> +static const unsigned int pciephy_v6_20_regs_layout[QPHY_LAYOUT_SIZE] = {
>>>> +       [QPHY_SW_RESET]                 = QPHY_V5_PCS_SW_RESET,
>>>> +       [QPHY_START_CTRL]               = QPHY_V5_PCS_START_CONTROL,
>>>> +       [QPHY_PCS_STATUS]               = QPHY_V5_PCS_PCS_STATUS1,
>>>> +       [QPHY_PCS_POWER_DOWN_CONTROL]   = QPHY_V5_PCS_POWER_DOWN_CONTROL,
>>>> +       [QPHY_PCS_ENDPOINT_REFCLK_CNTRL]        = QPHY_PCIE_V6_20_PCS_ENDPOINT_REFCLK_CNTRL,
>>>>    };
>>>>
>>>>    static const struct qmp_phy_init_tbl msm8998_pcie_serdes_tbl[] = {
>>>> @@ -2956,7 +2976,7 @@ static const struct qmp_phy_cfg sm8450_qmp_gen4x2_pciephy_cfg = {
>>>>           .num_resets             = ARRAY_SIZE(sdm845_pciephy_reset_l),
>>>>           .vreg_list              = qmp_phy_vreg_l,
>>>>           .num_vregs              = ARRAY_SIZE(qmp_phy_vreg_l),
>>>> -       .regs                   = pciephy_v5_regs_layout,
>>>> +       .regs                   = pciephy_v5_20_regs_layout,
>>>>
>>>>           .pwrdn_ctrl             = SW_PWRDN | REFCLK_DRV_DSBL,
>>>>           .phy_status             = PHYSTATUS_4_20,
>>>> @@ -3012,7 +3032,7 @@ static const struct qmp_phy_cfg sm8550_qmp_gen4x2_pciephy_cfg = {
>>>>           .num_resets             = ARRAY_SIZE(sdm845_pciephy_reset_l),
>>>>           .vreg_list              = sm8550_qmp_phy_vreg_l,
>>>>           .num_vregs              = ARRAY_SIZE(sm8550_qmp_phy_vreg_l),
>>>> -       .regs                   = pciephy_v5_regs_layout,
>>>> +       .regs                   = pciephy_v6_20_regs_layout,
>>>>
>>>>           .pwrdn_ctrl             = SW_PWRDN | REFCLK_DRV_DSBL,
>>>>           .phy_status             = PHYSTATUS_4_20,
>>>> @@ -3047,7 +3067,7 @@ static const struct qmp_phy_cfg sa8775p_qmp_gen4x2_pciephy_cfg = {
>>>>           .num_resets             = ARRAY_SIZE(sdm845_pciephy_reset_l),
>>>>           .vreg_list              = qmp_phy_vreg_l,
>>>>           .num_vregs              = ARRAY_SIZE(qmp_phy_vreg_l),
>>>> -       .regs                   = pciephy_v5_regs_layout,
>>>> +       .regs                   = pciephy_v5_20_regs_layout,
>>>>
>>>>           .pwrdn_ctrl             = SW_PWRDN | REFCLK_DRV_DSBL,
>>>>           .phy_status             = PHYSTATUS_4_20,
>>>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h
>>>> index a469ae2a10a1..9b166286afda 100644
>>>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h
>>>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h
>>>> @@ -11,6 +11,7 @@
>>>>    #define QPHY_V5_PCS_PCIE_POWER_STATE_CONFIG2           0x0c
>>>>    #define QPHY_V5_PCS_PCIE_POWER_STATE_CONFIG4           0x14
>>>>    #define QPHY_V5_PCS_PCIE_ENDPOINT_REFCLK_DRIVE         0x20
>>>> +#define QPHY_V5_PCS_PCIE_ENDPOINT_REFCLK_CNTRL         0x24
>>>>    #define QPHY_V5_PCS_PCIE_INT_AUX_CLK_CONFIG1           0x54
>>>>    #define QPHY_V5_PCS_PCIE_OSC_DTCT_ACTIONS              0x94
>>>>    #define QPHY_V5_PCS_PCIE_EQ_CONFIG2                    0xa8
>>>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h
>>>> index cdf8c04ea078..8b114e538a07 100644
>>>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h
>>>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5_20.h
>>>> @@ -9,6 +9,7 @@
>>>>    /* Only for QMP V5_20 PHY - PCIe PCS registers */
>>>>    #define QPHY_V5_20_PCS_PCIE_POWER_STATE_CONFIG2                0x00c
>>>>    #define QPHY_V5_20_PCS_PCIE_ENDPOINT_REFCLK_DRIVE      0x01c
>>>> +#define QPHY_V5_20_PCS_PCIE_ENDPOINT_REFCLK_CNTRL      0x020
>>>>    #define QPHY_V5_20_PCS_PCIE_OSC_DTCT_MODE2_CONFIG5     0x084
>>>>    #define QPHY_V5_20_PCS_PCIE_OSC_DTCT_ACTIONS           0x090
>>>>    #define QPHY_V5_20_PCS_PCIE_EQ_CONFIG1                 0x0a0
>>>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h
>>>> index e3eb08776339..f7abe95c49ad 100644
>>>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h
>>>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_20.h
>>>> @@ -10,6 +10,7 @@
>>>>    #define QPHY_PCIE_V6_20_PCS_POWER_STATE_CONFIG2                0x00c
>>>>    #define QPHY_PCIE_V6_20_PCS_TX_RX_CONFIG               0x018
>>>>    #define QPHY_PCIE_V6_20_PCS_ENDPOINT_REFCLK_DRIVE      0x01c
>>>> +#define QPHY_PCIE_V6_20_PCS_ENDPOINT_REFCLK_CNTRL      0x020
>>>>    #define QPHY_PCIE_V6_20_PCS_OSC_DTCT_ATCIONS           0x090
>>>>    #define QPHY_PCIE_V6_20_PCS_EQ_CONFIG1                 0x0a0
>>>>    #define QPHY_PCIE_V6_20_PCS_EQ_CONFIG5                 0x108
>>>>
>>>> --
>>>> 2.42.0
>>>>
>>>>
>
>

Re: [PATCH v2 0/3] phy: qcom-qmp-pcie: Add support to keep refclk always on

[Konrad Dybcio]

On 11/9/23 10:32, Krishna Chaitanya Chundru wrote:
> 
> On 11/8/2023 3:27 AM, Konrad Dybcio wrote:
>>
>>
>> On 11/7/23 13:26, Krishna chaitanya chundru wrote:
>>> This series adds support to provide refclk to endpoint even in low
>>> power states.
>>>
>>> Due to some platform specific issues with CLKREQ signal, it is not being
>>> propagated to the host and as host doesn't know the clkreq signal host is
>>> not sending refclk. Due to this endpoint is seeing linkdown and going
>>> to bad state.
>>> To avoid those ref clk should be provided always to the endpoint. The
>>> issue is coming only when ep intiates the L1.1 or L1.2 exit and clkreq
>>> is not being propagated properly to the host.
>> I'm gonna sound like a broken record, but:
>>
>> How much power does this consume? Would it matter if we kept this
>> clock always-on for all platforms with this version of the phy?
>>
>> Konrad
> 
> We see about 22mw extra power consumption with refclk always on.
> 
> We can't keep this property always on as there is impact on power consumption.
Ooeheh, that's not far away from a low-clocked efficiency CPU core..

Thanks for checking!

Konrad

Re: [PATCH v2 1/3] dt-bindings: phy: qcom,qmp: Add PCIe qcom,refclk-always-on property

[Krishna Chaitanya Chundru]

On 11/8/2023 9:22 PM, Conor Dooley wrote:
> On Tue, Nov 07, 2023 at 03:01:47PM +0200, Dmitry Baryshkov wrote:
>> On Tue, 7 Nov 2023 at 14:26, Krishna chaitanya chundru
>> <quic_krichai@quicinc.com> wrote:
>>> Document qcom,refclk-always-on property which is needed in some platforms
>>> to supply refclk even in PCIe low power states.
>>>
>>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>>> ---
>>>   .../devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml        | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
>>> index 2c3d6553a7ba..263291447a5b 100644
>>> --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
>>> +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
>>> @@ -93,6 +93,13 @@ properties:
>>>     "#phy-cells":
>>>       const: 0
>>>
>>> +  qcom,refclk-always-on:
>>> +    type: boolean
>>> +    description: If there is some issues in platform with clkreq signal
>> nit: there are some
>>
>> However this still doesn't describe what kind of issues with clkreq
>> you observe. I mean, clkreq is just a GPIO pin.
>>
>>> +      propagation to the host and due to that host will not send refclk, which
>>> +      results in linkdown in L1.2 or L1.1 exit initiated by EP. This property
>>> +      if set keeps refclk always on even in Low power states (optional).
> Dimitry's issues with the property aside, putting "(optional)" in the
> description is meaningless - qcom,refclk-always-on isn't listed in the
> required properties section, so therefore has to be optional.
>
> Cheers,
> Conor.

I removed the optional flag and updated the description.

- Krishna chaitanya.

>>> +
>>>   required:
>>>     - compatible
>>>     - reg
>>>
>>> --
>>> 2.42.0
>>>
>>>
>>
>> -- 
>> With best wishes
>> Dmitry