[PATCH 0/2] phy: qcom: qmp-pcie: Add support for Gen4 4-lane mode for X1E80100

[PATCH 0/2] phy: qcom: qmp-pcie: Add support for Gen4 4-lane mode for X1E80100

[Abel Vesa]

On both QCP and CRD board currently supported upstream, the NVMe sits
on the PCIe 6. Until now that has been configured in dual lane mode
only. The schematics reveal that the NVMe is actually using 4 lanes.
So add support for the 4-lane mode and document the compatible for it.

Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
Abel Vesa (2):
      dt-bindings: phy: qcom,sc8280xp-qmp-pcie-phy: Document the X1E80100 QMP PCIe PHY Gen4 x4
      phy: qcom: qmp-pcie: Add X1E80100 Gen4 4-lane mode support

 .../bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml   |  3 ++
 drivers/phy/qualcomm/phy-qcom-qmp-pcie.c           | 42 ++++++++++++++++++++++
 2 files changed, 45 insertions(+)
---
base-commit: 0e1980c40b6edfa68b6acf926bab22448a6e40c9
change-id: 20240531-x1e80100-phy-add-gen4x4-fa830a5505b6

Best regards,
-- 
Abel Vesa <abel.vesa@linaro.org>

[PATCH 1/2] dt-bindings: phy: qcom,sc8280xp-qmp-pcie-phy: Document the X1E80100 QMP PCIe PHY Gen4 x4

[Abel Vesa]

The PCIe 6th instance from X1E80100 can be used in both 4-lane mode or
2-lane mode. Document the 4-lane mode as a separate compatible.

Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
 Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml | 3 +++
 1 file changed, 3 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 16634f73bdcf..f96f692c9ee5 100644
--- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
@@ -40,6 +40,7 @@ properties:
       - qcom,sm8650-qmp-gen4x2-pcie-phy
       - qcom,x1e80100-qmp-gen3x2-pcie-phy
       - qcom,x1e80100-qmp-gen4x2-pcie-phy
+      - qcom,x1e80100-qmp-gen4x4-pcie-phy
 
   reg:
     minItems: 1
@@ -119,6 +120,7 @@ allOf:
           contains:
             enum:
               - qcom,sc8280xp-qmp-gen3x4-pcie-phy
+              - qcom,x1e80100-qmp-gen4x4-pcie-phy
     then:
       properties:
         reg:
@@ -170,6 +172,7 @@ allOf:
               - qcom,sc8280xp-qmp-gen3x1-pcie-phy
               - qcom,sc8280xp-qmp-gen3x2-pcie-phy
               - qcom,sc8280xp-qmp-gen3x4-pcie-phy
+              - qcom,x1e80100-qmp-gen4x4-pcie-phy
     then:
       properties:
         clocks:

-- 
2.34.1

Re: [PATCH 1/2] dt-bindings: phy: qcom,sc8280xp-qmp-pcie-phy: Document the X1E80100 QMP PCIe PHY Gen4 x4

[Krzysztof Kozlowski]

On 31/05/2024 18:06, Abel Vesa wrote:
> The PCIe 6th instance from X1E80100 can be used in both 4-lane mode or
> 2-lane mode. Document the 4-lane mode as a separate compatible.
> 
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>


Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: phy: qcom,sc8280xp-qmp-pcie-phy: Document the X1E80100 QMP PCIe PHY Gen4 x4

[Johan Hovold]

On Fri, May 31, 2024 at 07:06:44PM +0300, Abel Vesa wrote:
> The PCIe 6th instance from X1E80100 can be used in both 4-lane mode or

nit: s/PCIe 6th/sixth PCIe/
nit: s/from/on/
nit: s/both/either/

> 2-lane mode. Document the 4-lane mode as a separate compatible.
> 
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>

Johan

Re: [PATCH 1/2] dt-bindings: phy: qcom,sc8280xp-qmp-pcie-phy: Document the X1E80100 QMP PCIe PHY Gen4 x4

[Krzysztof Kozlowski]

On 03/06/2024 14:43, Johan Hovold wrote:
> On Fri, May 31, 2024 at 07:06:44PM +0300, Abel Vesa wrote:
>> The PCIe 6th instance from X1E80100 can be used in both 4-lane mode or
> 
> nit: s/PCIe 6th/sixth PCIe/
> nit: s/from/on/
> nit: s/both/either/

That's really nit-picking and not helpful in getting things merged.

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: phy: qcom,sc8280xp-qmp-pcie-phy: Document the X1E80100 QMP PCIe PHY Gen4 x4

[Johan Hovold]

On Tue, Jun 04, 2024 at 01:00:35PM +0200, Krzysztof Kozlowski wrote:
> On 03/06/2024 14:43, Johan Hovold wrote:
> > On Fri, May 31, 2024 at 07:06:44PM +0300, Abel Vesa wrote:
> >> The PCIe 6th instance from X1E80100 can be used in both 4-lane mode or
> > 
> > nit: s/PCIe 6th/sixth PCIe/
> > nit: s/from/on/
> > nit: s/both/either/
> 
> That's really nit-picking and not helpful in getting things merged.

Writing proper commit messages with correct grammar is in no way to be
asking too much (especially from experienced kernel devs).

Also, the code does not work so it should not be merged yet anyway.

Johan

Re: [PATCH 1/2] dt-bindings: phy: qcom,sc8280xp-qmp-pcie-phy: Document the X1E80100 QMP PCIe PHY Gen4 x4

[Krzysztof Kozlowski]

On 04/06/2024 13:17, Johan Hovold wrote:
> On Tue, Jun 04, 2024 at 01:00:35PM +0200, Krzysztof Kozlowski wrote:
>> On 03/06/2024 14:43, Johan Hovold wrote:
>>> On Fri, May 31, 2024 at 07:06:44PM +0300, Abel Vesa wrote:
>>>> The PCIe 6th instance from X1E80100 can be used in both 4-lane mode or
>>>
>>> nit: s/PCIe 6th/sixth PCIe/
>>> nit: s/from/on/
>>> nit: s/both/either/
>>
>> That's really nit-picking and not helpful in getting things merged.
> 
> Writing proper commit messages with correct grammar is in no way to be
> asking too much (especially from experienced kernel devs).

Well, many of us are not native English speakers, so when the commit msg
is 100% understandable, pointing out grammar mistakes or style
(6th->sixth) issues is nit-picking and is asking for unnecessary work.

> 
> Also, the code does not work so it should not be merged yet anyway.

OK

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: phy: qcom,sc8280xp-qmp-pcie-phy: Document the X1E80100 QMP PCIe PHY Gen4 x4

[Johan Hovold]

On Tue, Jun 04, 2024 at 01:37:46PM +0200, Krzysztof Kozlowski wrote:
> On 04/06/2024 13:17, Johan Hovold wrote:
> > On Tue, Jun 04, 2024 at 01:00:35PM +0200, Krzysztof Kozlowski wrote:
> >> On 03/06/2024 14:43, Johan Hovold wrote:
> >>> On Fri, May 31, 2024 at 07:06:44PM +0300, Abel Vesa wrote:
> >>>> The PCIe 6th instance from X1E80100 can be used in both 4-lane mode or
> >>>
> >>> nit: s/PCIe 6th/sixth PCIe/
> >>> nit: s/from/on/
> >>> nit: s/both/either/
> >>
> >> That's really nit-picking and not helpful in getting things merged.
> > 
> > Writing proper commit messages with correct grammar is in no way to be
> > asking too much (especially from experienced kernel devs).
> 
> Well, many of us are not native English speakers, so when the commit msg
> is 100% understandable, pointing out grammar mistakes or style
> (6th->sixth) issues is nit-picking and is asking for unnecessary work.

Well I did use a "nit:" prefix for a reason. And I hope people are still
interested in improving their English even if it's not their first
language.

Johan

Re: [PATCH 1/2] dt-bindings: phy: qcom,sc8280xp-qmp-pcie-phy: Document the X1E80100 QMP PCIe PHY Gen4 x4

[Abel Vesa]

On 24-06-03 14:43:15, Johan Hovold wrote:
> On Fri, May 31, 2024 at 07:06:44PM +0300, Abel Vesa wrote:
> > The PCIe 6th instance from X1E80100 can be used in both 4-lane mode or
> 
> nit: s/PCIe 6th/sixth PCIe/
> nit: s/from/on/
> nit: s/both/either/
> 

Thanks for reviewing. Will address them in the next version.

> > 2-lane mode. Document the 4-lane mode as a separate compatible.
> > 
> > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> 
> Johan

[PATCH 2/2] phy: qcom: qmp-pcie: Add X1E80100 Gen4 4-lane mode support

[Abel Vesa]

The PCIe 6th instance from X1E80100 can be used in both 4-lane mode or
2-lane mode. Add the configuration and compatible for the 4-lane mode.

Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
 drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 42 ++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
index 6c796723c8f5..4e0b28da69a7 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
@@ -1028,6 +1028,10 @@ static const struct qmp_phy_init_tbl x1e80100_qmp_gen4x2_pcie_ln_shrd_tbl[] = {
 	QMP_PHY_INIT_CFG(QSERDES_V6_LN_SHRD_RX_MARG_COARSE_THRESH6_RATE3, 0x1f),
 };
 
+static const struct qmp_phy_init_tbl x1e80100_qmp_gen4x4_pcie_serdes_4ln_tbl[] = {
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_BIAS_EN_CLK_BUFLR_EN, 0x1c),
+};
+
 static const struct qmp_phy_init_tbl x1e80100_qmp_gen4x2_pcie_tx_tbl[] = {
 	QMP_PHY_INIT_CFG(QSERDES_V6_20_TX_RES_CODE_LANE_OFFSET_TX, 0x1d),
 	QMP_PHY_INIT_CFG(QSERDES_V6_20_TX_RES_CODE_LANE_OFFSET_RX, 0x03),
@@ -3342,6 +3346,41 @@ static const struct qmp_phy_cfg x1e80100_qmp_gen4x2_pciephy_cfg = {
 		.ln_shrd		= x1e80100_qmp_gen4x2_pcie_ln_shrd_tbl,
 		.ln_shrd_num		= ARRAY_SIZE(x1e80100_qmp_gen4x2_pcie_ln_shrd_tbl),
 	},
+
+	.reset_list		= sdm845_pciephy_reset_l,
+	.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_v6_regs_layout,
+
+	.pwrdn_ctrl		= SW_PWRDN | REFCLK_DRV_DSBL,
+	.phy_status		= PHYSTATUS_4_20,
+	.has_nocsr_reset	= true,
+};
+
+static const struct qmp_phy_cfg x1e80100_qmp_gen4x4_pciephy_cfg = {
+	.lanes = 4,
+
+	.offsets		= &qmp_pcie_offsets_v6_20,
+
+	.tbls = {
+		.serdes			= x1e80100_qmp_gen4x2_pcie_serdes_tbl,
+		.serdes_num		= ARRAY_SIZE(x1e80100_qmp_gen4x2_pcie_serdes_tbl),
+		.tx			= x1e80100_qmp_gen4x2_pcie_tx_tbl,
+		.tx_num			= ARRAY_SIZE(x1e80100_qmp_gen4x2_pcie_tx_tbl),
+		.rx			= x1e80100_qmp_gen4x2_pcie_rx_tbl,
+		.rx_num			= ARRAY_SIZE(x1e80100_qmp_gen4x2_pcie_rx_tbl),
+		.pcs			= x1e80100_qmp_gen4x2_pcie_pcs_tbl,
+		.pcs_num		= ARRAY_SIZE(x1e80100_qmp_gen4x2_pcie_pcs_tbl),
+		.pcs_misc		= x1e80100_qmp_gen4x2_pcie_pcs_misc_tbl,
+		.pcs_misc_num		= ARRAY_SIZE(x1e80100_qmp_gen4x2_pcie_pcs_misc_tbl),
+		.ln_shrd		= x1e80100_qmp_gen4x2_pcie_ln_shrd_tbl,
+		.ln_shrd_num		= ARRAY_SIZE(x1e80100_qmp_gen4x2_pcie_ln_shrd_tbl),
+	},
+
+	.serdes_4ln_tbl		= x1e80100_qmp_gen4x4_pcie_serdes_4ln_tbl,
+	.serdes_4ln_num		= ARRAY_SIZE(x1e80100_qmp_gen4x4_pcie_serdes_4ln_tbl),
+
 	.reset_list		= sdm845_pciephy_reset_l,
 	.num_resets		= ARRAY_SIZE(sdm845_pciephy_reset_l),
 	.vreg_list		= sm8550_qmp_phy_vreg_l,
@@ -4108,6 +4147,9 @@ static const struct of_device_id qmp_pcie_of_match_table[] = {
 	}, {
 		.compatible = "qcom,x1e80100-qmp-gen4x2-pcie-phy",
 		.data = &x1e80100_qmp_gen4x2_pciephy_cfg,
+	}, {
+		.compatible = "qcom,x1e80100-qmp-gen4x4-pcie-phy",
+		.data = &x1e80100_qmp_gen4x4_pciephy_cfg,
 	},
 	{ },
 };

-- 
2.34.1

Re: [PATCH 2/2] phy: qcom: qmp-pcie: Add X1E80100 Gen4 4-lane mode support

[Dmitry Baryshkov]

On Fri, May 31, 2024 at 07:06:45PM +0300, Abel Vesa wrote:
> The PCIe 6th instance from X1E80100 can be used in both 4-lane mode or
> 2-lane mode. Add the configuration and compatible for the 4-lane mode.
> 
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 42 ++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 

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


-- 
With best wishes
Dmitry

Re: [PATCH 2/2] phy: qcom: qmp-pcie: Add X1E80100 Gen4 4-lane mode support

[Johan Hovold]

On Fri, May 31, 2024 at 07:06:45PM +0300, Abel Vesa wrote:
> The PCIe 6th instance from X1E80100 can be used in both 4-lane mode or
> 2-lane mode. Add the configuration and compatible for the 4-lane mode.

Same language nits as for patch 1/1.

> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>

I tried this patch along with the DT changes and the link on the CRD
still comes up as 2-lane:

	qcom-pcie 1bf8000.pci: PCIe Gen.4 x2 link up

so something appears to be wrong here. (I noticed the same with your
next branch last week.)

How did you test this? Does the link actually come up as 4-lane for you?

Johan

Re: [PATCH 2/2] phy: qcom: qmp-pcie: Add X1E80100 Gen4 4-lane mode support

[Abel Vesa]

On 24-06-03 14:46:12, Johan Hovold wrote:
> On Fri, May 31, 2024 at 07:06:45PM +0300, Abel Vesa wrote:
> > The PCIe 6th instance from X1E80100 can be used in both 4-lane mode or
> > 2-lane mode. Add the configuration and compatible for the 4-lane mode.
> 
> Same language nits as for patch 1/1.
> 
> > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> 
> I tried this patch along with the DT changes and the link on the CRD
> still comes up as 2-lane:
> 
> 	qcom-pcie 1bf8000.pci: PCIe Gen.4 x2 link up
> 
> so something appears to be wrong here. (I noticed the same with your
> next branch last week.)
> 
> How did you test this? Does the link actually come up as 4-lane for you?

This is the PHY part. The controller needs some changes as well.

Yes, as of yet, I'm not able to bring the link up in 4-lanes mode.
This however doesn't mean the PHY sequence is incorrect.

But, I agree, maybe I should hold on to the PHY changes as well until
we get the controller side working as well.

Thanks for reviewing.

> 
> Johan

Re: [PATCH 2/2] phy: qcom: qmp-pcie: Add X1E80100 Gen4 4-lane mode support

[Johan Hovold]

On Tue, Jun 04, 2024 at 03:25:52PM +0300, Abel Vesa wrote:
> On 24-06-03 14:46:12, Johan Hovold wrote:

> > I tried this patch along with the DT changes and the link on the CRD
> > still comes up as 2-lane:
> > 
> > 	qcom-pcie 1bf8000.pci: PCIe Gen.4 x2 link up
> > 
> > so something appears to be wrong here. (I noticed the same with your
> > next branch last week.)
> > 
> > How did you test this? Does the link actually come up as 4-lane for you?
> 
> This is the PHY part. The controller needs some changes as well.

Are you sure? It wasn't needed on sc8280xp (except for updating
num-lanes in DT).

> Yes, as of yet, I'm not able to bring the link up in 4-lanes mode.

Thanks for confirming.

> This however doesn't mean the PHY sequence is incorrect.

Not necessarily, but it means it hasn't been fully tested and that it
could potentially be the reason for the failed x4 link up.

Johan