[PATCH 0/8] Add support for PCIe3 on x1e80100

[PATCH 0/8] Add support for PCIe3 on x1e80100

[Qiang Yu]

This series add support for PCIe3 on x1e80100.

PCIe3 needs additional set of clocks, regulators and new set of PCIe QMP
PHY configuration compare other PCIe instances on x1e80100. Hence add
required resource configuration and usage for PCIe3.

Qiang Yu (8):
  phy: qcom-qmp: pcs-pcie: Add v6.30 register offsets
  phy: qcom-qmp: pcs: Add v6.30 register offsets
  phy: qcom: qmp: Add phy register and clk setting for x1e80100 PCIe3
  arm64: dts: qcom: x1e80100: Add support for PCIe3 on x1e80100
  dt-bindings: phy: qcom,sc8280xp-qmp-pcie-phy: Document the X1E80100
    QMP PCIe PHY Gen4 x8
  clk: qcom: gcc-x1e80100: Fix halt_check for pipediv2 clocks
  arm64: dts: qcom: x1e80100-qcp: Add power supply and sideband signal
    for pcie3
  PCI: qcom: Add support to PCIe slot power supplies

 .../phy/qcom,sc8280xp-qmp-pcie-phy.yaml       |  18 +-
 arch/arm64/boot/dts/qcom/x1e80100-qcp.dts     | 116 +++++++++
 arch/arm64/boot/dts/qcom/x1e80100.dtsi        | 205 +++++++++++++++-
 drivers/clk/qcom/gcc-x1e80100.c               |  10 +-
 drivers/pci/controller/dwc/pcie-qcom.c        |  52 +++-
 drivers/phy/qualcomm/phy-qcom-qmp-pcie.c      | 222 +++++++++++++++++-
 .../qualcomm/phy-qcom-qmp-pcs-pcie-v6_30.h    |  25 ++
 drivers/phy/qualcomm/phy-qcom-qmp-pcs-v6_30.h |  19 ++
 8 files changed, 657 insertions(+), 10 deletions(-)
 create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_30.h
 create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-pcs-v6_30.h

-- 
2.34.1

[PATCH 1/8] phy: qcom-qmp: pcs-pcie: Add v6.30 register offsets

[Qiang Yu]

x1e80100 SoC uses QMP phy with version v6.30 for PCIe Gen4 x8. Add the new
PCS PCIE specific offsets in a dedicated header file.

Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
---
 .../qualcomm/phy-qcom-qmp-pcs-pcie-v6_30.h    | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_30.h

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_30.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_30.h
new file mode 100644
index 000000000000..5a58ff197e6e
--- /dev/null
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_30.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2024 Qualcomm Innovation Center. All rights reserved.
+ */
+
+#ifndef QCOM_PHY_QMP_PCS_PCIE_V6_30_H_
+#define QCOM_PHY_QMP_PCS_PCIE_V6_30_H_
+
+/* Only for QMP V6_30 PHY - PCIE have different offsets than V6 */
+#define QPHY_PCIE_V6_30_PCS_POWER_STATE_CONFIG2		0x014
+#define QPHY_PCIE_V6_30_PCS_TX_RX_CONFIG		0x020
+#define QPHY_PCIE_V6_30_PCS_ENDPOINT_REFCLK_DRIVE	0x024
+#define QPHY_PCIE_V6_30_PCS_OSC_DTCT_ACTIONS		0x098
+#define QPHY_PCIE_V6_30_PCS_EQ_CONFIG1			0x0a8
+#define QPHY_PCIE_V6_30_PCS_G3_RXEQEVAL_TIME		0x0f8
+#define QPHY_PCIE_V6_30_PCS_G4_RXEQEVAL_TIME		0x0fc
+#define QPHY_PCIE_V6_30_PCS_G4_EQ_CONFIG5		0x110
+#define QPHY_PCIE_V6_30_PCS_G4_PRE_GAIN			0x164
+#define QPHY_PCIE_V6_30_PCS_RX_MARGINING_CONFIG1	0x184
+#define QPHY_PCIE_V6_30_PCS_RX_MARGINING_CONFIG3	0x18c
+#define QPHY_PCIE_V6_30_PCS_RX_MARGINING_CONFIG5	0x194
+#define QPHY_PCIE_V6_30_PCS_G3_FOM_EQ_CONFIG5		0x1b4
+#define QPHY_PCIE_V6_30_PCS_G4_FOM_EQ_CONFIG5		0x1c8
+
+#endif
-- 
2.34.1

Re: [PATCH 1/8] phy: qcom-qmp: pcs-pcie: Add v6.30 register offsets

[Krzysztof Kozlowski]

On Mon, Aug 26, 2024 at 11:36:24PM -0700, Qiang Yu wrote:
> x1e80100 SoC uses QMP phy with version v6.30 for PCIe Gen4 x8. Add the new
> PCS PCIE specific offsets in a dedicated header file.
> 
> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> ---
>  .../qualcomm/phy-qcom-qmp-pcs-pcie-v6_30.h    | 25 +++++++++++++++++++
>  1 file changed, 25 insertions(+)
>  create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_30.h
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_30.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_30.h
> new file mode 100644
> index 000000000000..5a58ff197e6e
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_30.h
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2024 Qualcomm Innovation Center. All rights reserved.
> + */
> +
> +#ifndef QCOM_PHY_QMP_PCS_PCIE_V6_30_H_
> +#define QCOM_PHY_QMP_PCS_PCIE_V6_30_H_
> +
> +/* Only for QMP V6_30 PHY - PCIE have different offsets than V6 */
> +#define QPHY_PCIE_V6_30_PCS_POWER_STATE_CONFIG2		0x014
> +#define QPHY_PCIE_V6_30_PCS_TX_RX_CONFIG		0x020
> +#define QPHY_PCIE_V6_30_PCS_ENDPOINT_REFCLK_DRIVE	0x024
> +#define QPHY_PCIE_V6_30_PCS_OSC_DTCT_ACTIONS		0x098
> +#define QPHY_PCIE_V6_30_PCS_EQ_CONFIG1			0x0a8
> +#define QPHY_PCIE_V6_30_PCS_G3_RXEQEVAL_TIME		0x0f8
> +#define QPHY_PCIE_V6_30_PCS_G4_RXEQEVAL_TIME		0x0fc
> +#define QPHY_PCIE_V6_30_PCS_G4_EQ_CONFIG5		0x110
> +#define QPHY_PCIE_V6_30_PCS_G4_PRE_GAIN			0x164
> +#define QPHY_PCIE_V6_30_PCS_RX_MARGINING_CONFIG1	0x184
> +#define QPHY_PCIE_V6_30_PCS_RX_MARGINING_CONFIG3	0x18c
> +#define QPHY_PCIE_V6_30_PCS_RX_MARGINING_CONFIG5	0x194
> +#define QPHY_PCIE_V6_30_PCS_G3_FOM_EQ_CONFIG5		0x1b4
> +#define QPHY_PCIE_V6_30_PCS_G4_FOM_EQ_CONFIG5		0x1c8

There is no user of these. Squash it with the user, because there is
little point in adding dead code.

Best regards,
Krzysztof

Re: [PATCH 1/8] phy: qcom-qmp: pcs-pcie: Add v6.30 register offsets

[Qiang Yu]

On 8/27/2024 7:37 PM, Krzysztof Kozlowski wrote:
> On Mon, Aug 26, 2024 at 11:36:24PM -0700, Qiang Yu wrote:
>> x1e80100 SoC uses QMP phy with version v6.30 for PCIe Gen4 x8. Add the new
>> PCS PCIE specific offsets in a dedicated header file.
>>
>> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
>> ---
>>   .../qualcomm/phy-qcom-qmp-pcs-pcie-v6_30.h    | 25 +++++++++++++++++++
>>   1 file changed, 25 insertions(+)
>>   create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_30.h
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_30.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_30.h
>> new file mode 100644
>> index 000000000000..5a58ff197e6e
>> --- /dev/null
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_30.h
>> @@ -0,0 +1,25 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright (c) 2024 Qualcomm Innovation Center. All rights reserved.
>> + */
>> +
>> +#ifndef QCOM_PHY_QMP_PCS_PCIE_V6_30_H_
>> +#define QCOM_PHY_QMP_PCS_PCIE_V6_30_H_
>> +
>> +/* Only for QMP V6_30 PHY - PCIE have different offsets than V6 */
>> +#define QPHY_PCIE_V6_30_PCS_POWER_STATE_CONFIG2		0x014
>> +#define QPHY_PCIE_V6_30_PCS_TX_RX_CONFIG		0x020
>> +#define QPHY_PCIE_V6_30_PCS_ENDPOINT_REFCLK_DRIVE	0x024
>> +#define QPHY_PCIE_V6_30_PCS_OSC_DTCT_ACTIONS		0x098
>> +#define QPHY_PCIE_V6_30_PCS_EQ_CONFIG1			0x0a8
>> +#define QPHY_PCIE_V6_30_PCS_G3_RXEQEVAL_TIME		0x0f8
>> +#define QPHY_PCIE_V6_30_PCS_G4_RXEQEVAL_TIME		0x0fc
>> +#define QPHY_PCIE_V6_30_PCS_G4_EQ_CONFIG5		0x110
>> +#define QPHY_PCIE_V6_30_PCS_G4_PRE_GAIN			0x164
>> +#define QPHY_PCIE_V6_30_PCS_RX_MARGINING_CONFIG1	0x184
>> +#define QPHY_PCIE_V6_30_PCS_RX_MARGINING_CONFIG3	0x18c
>> +#define QPHY_PCIE_V6_30_PCS_RX_MARGINING_CONFIG5	0x194
>> +#define QPHY_PCIE_V6_30_PCS_G3_FOM_EQ_CONFIG5		0x1b4
>> +#define QPHY_PCIE_V6_30_PCS_G4_FOM_EQ_CONFIG5		0x1c8
> There is no user of these. Squash it with the user, because there is
> little point in adding dead code.
>
> Best regards,
> Krzysztof
OK, will squash this three patches related to phy setting into one patch.

Thanks,
Qiang

[PATCH 2/8] phy: qcom-qmp: pcs: Add v6.30 register offsets

[Qiang Yu]

x1e80100 SoC uses QMP phy with version v6.30 for PCIe Gen4 x8. Add the new
PCS offsets in a dedicated header file.

Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-pcs-v6_30.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-pcs-v6_30.h

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-v6_30.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-v6_30.h
new file mode 100644
index 000000000000..9aa6d3622c24
--- /dev/null
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-v6_30.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2024 Qualcomm Innovation Center. All rights reserved.
+ */
+
+#ifndef QCOM_PHY_QMP_PCS_V6_30_H_
+#define QCOM_PHY_QMP_PCS_V6_30_H_
+
+/* Only for QMP V6_30 PHY - PCIe PCS registers */
+#define	QPHY_V6_30_PCS_LOCK_DETECT_CONFIG2		0x0cc
+#define	QPHY_V6_30_PCS_G3S2_PRE_GAIN			0x17c
+#define	QPHY_V6_30_PCS_RX_SIGDET_LVL			0x194
+#define	QPHY_V6_30_PCS_ALIGN_DETECT_CONFIG7		0x1dc
+#define	QPHY_V6_30_PCS_TX_RX_CONFIG			0x1e0
+#define	QPHY_V6_30_PCS_TX_RX_CONFIG2			0x1e4
+#define	QPHY_V6_30_PCS_EQ_CONFIG4			0x1fc
+#define	QPHY_V6_30_PCS_EQ_CONFIG5			0x200
+
+#endif
-- 
2.34.1

Re: [PATCH 2/8] phy: qcom-qmp: pcs: Add v6.30 register offsets

[Konrad Dybcio]

On 27.08.2024 8:36 AM, Qiang Yu wrote:
> x1e80100 SoC uses QMP phy with version v6.30 for PCIe Gen4 x8. Add the new
> PCS offsets in a dedicated header file.
> 
> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-pcs-v6_30.h | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>  create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-pcs-v6_30.h
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-v6_30.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-v6_30.h
> new file mode 100644
> index 000000000000..9aa6d3622c24
> --- /dev/null
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-v6_30.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2024 Qualcomm Innovation Center. All rights reserved.
> + */
> +
> +#ifndef QCOM_PHY_QMP_PCS_V6_30_H_
> +#define QCOM_PHY_QMP_PCS_V6_30_H_
> +
> +/* Only for QMP V6_30 PHY - PCIe PCS registers */
> +#define	QPHY_V6_30_PCS_LOCK_DETECT_CONFIG2		0x0cc
> +#define	QPHY_V6_30_PCS_G3S2_PRE_GAIN			0x17c
> +#define	QPHY_V6_30_PCS_RX_SIGDET_LVL			0x194
> +#define	QPHY_V6_30_PCS_ALIGN_DETECT_CONFIG7		0x1dc
> +#define	QPHY_V6_30_PCS_TX_RX_CONFIG			0x1e0
> +#define	QPHY_V6_30_PCS_TX_RX_CONFIG2			0x1e4
> +#define	QPHY_V6_30_PCS_EQ_CONFIG4			0x1fc
> +#define	QPHY_V6_30_PCS_EQ_CONFIG5			0x200

Squash with the previous one and the next one, and please make the
indentation consistent

Konrad

[PATCH 3/8] phy: qcom: qmp: Add phy register and clk setting for x1e80100 PCIe3

[Qiang Yu]

Currently driver supports only x4 lane based functionality using tx/rx and
tx2/rx2 pair of register sets. To support 8 lane functionality with PCIe3,
PCIe3 related QMP PHY provides additional programming which are available
as txz and rxz based register set. Hence adds txz and rxz based registers
usage and programming sequences. Phy register setting for txz and rxz will
be applied to all 8 lanes. Some lanes may have different settings on
several registers than txz/rxz, these registers should be programmed after
txz/rxz programming sequences completing.

Besides, PCIe3 related QMP PHY also requires addtional clk, which is named
as clkref_en. Hence, add this clk into qmp_pciephy_clk_l so that it can be
easily parsed from devicetree during init.

Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 222 ++++++++++++++++++++++-
 1 file changed, 221 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
index 4b4a10f7f6d6..17ee30e04aea 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
@@ -34,6 +34,8 @@
 #include "phy-qcom-qmp-pcs-pcie-v5_20.h"
 #include "phy-qcom-qmp-pcs-pcie-v6.h"
 #include "phy-qcom-qmp-pcs-pcie-v6_20.h"
+#include "phy-qcom-qmp-pcs-pcie-v6_30.h"
+#include "phy-qcom-qmp-pcs-v6_30.h"
 #include "phy-qcom-qmp-pcie-qhp.h"
 
 #define PHY_INIT_COMPLETE_TIMEOUT		10000
@@ -1340,6 +1342,155 @@ static const struct qmp_phy_init_tbl x1e80100_qmp_gen4x2_pcie_pcs_misc_tbl[] = {
 	QMP_PHY_INIT_CFG(QPHY_PCIE_V6_20_PCS_G4_FOM_EQ_CONFIG5, 0x8a),
 };
 
+static const struct qmp_phy_init_tbl x1e80100_qmp_gen4x8_pcie_txz_tbl[] = {
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_TX_RES_CODE_LANE_OFFSET_TX, 0x1a),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_TX_RES_CODE_LANE_OFFSET_RX, 0x05),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_TX_LANE_MODE_1, 0x01),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_TX_LANE_MODE_2, 0x10),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_TX_LANE_MODE_3, 0x51),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_TX_TRAN_DRVR_EMP_EN, 0x34),
+};
+
+static const struct qmp_phy_init_tbl x1e80100_qmp_gen4x8_pcie_rxz_tbl[] = {
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_UCDR_FO_GAIN_RATE_2, 0x0c),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_UCDR_SO_GAIN_RATE_2, 0x04),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_UCDR_FO_GAIN_RATE_3, 0x0a),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_UCDR_PI_CONTROLS, 0x16),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_UCDR_SO_ACC_DEFAULT_VAL_RATE3, 0x00),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_IVCM_CAL_CTRL2, 0x80),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_IVCM_POSTCAL_OFFSET, 0x00),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_BKUP_CTRL1, 0x15),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_DFE_3, 0x45),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_VGA_CAL_MAN_VAL, 0x0c),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_VGA_CAL_CNTRL1, 0x00),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_GM_CAL, 0x0d),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_EQU_ADAPTOR_CNTRL4, 0x0b),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_SIGDET_ENABLES, 0x1c),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_PHPRE_CTRL, 0x20),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_DFE_CTLE_POST_CAL_OFFSET, 0x38),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_Q_PI_INTRINSIC_BIAS_RATE32, 0x39),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_MODE_RATE2_B0, 0xd4),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_MODE_RATE2_B1, 0x23),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_MODE_RATE2_B2, 0x58),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_MODE_RATE2_B3, 0x9a),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_MODE_RATE2_B4, 0x38),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_MODE_RATE2_B5, 0xb6),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_MODE_RATE2_B6, 0xee),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_MODE_RATE3_B0, 0x1c),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_MODE_RATE3_B1, 0xe4),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_MODE_RATE3_B2, 0x60),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_MODE_RATE3_B3, 0xdf),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_MODE_RATE3_B4, 0x69),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_MODE_RATE3_B5, 0x76),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_MODE_RATE3_B6, 0xff),
+	QMP_PHY_INIT_CFG(QSERDES_V6_20_RX_TX_ADPT_CTRL, 0x10),
+};
+
+static const struct qmp_phy_init_tbl x1e80100_qmp_gen4x8_pcie_rx_tbl[] = {
+	QMP_PHY_INIT_CFG_LANE(QSERDES_V6_20_RX_DFE_CTLE_POST_CAL_OFFSET, 0x3a, 1),
+};
+
+static const struct qmp_phy_init_tbl x1e80100_qmp_gen4x8_pcie_ln_shrd_tbl[] = {
+	QMP_PHY_INIT_CFG(QSERDES_V6_LN_SHRD_RXCLK_DIV2_CTRL, 0x01),
+	QMP_PHY_INIT_CFG(QSERDES_V6_LN_SHRD_RX_SUMMER_CAL_SPD_MODE, 0x5b),
+	QMP_PHY_INIT_CFG(QSERDES_V6_LN_SHRD_DFE_DAC_ENABLE1, 0x88),
+	QMP_PHY_INIT_CFG(QSERDES_V6_LN_SHRD_TX_ADAPT_POST_THRESH1, 0x02),
+	QMP_PHY_INIT_CFG(QSERDES_V6_LN_SHRD_TX_ADAPT_POST_THRESH2, 0x0d),
+	QMP_PHY_INIT_CFG(QSERDES_V6_LN_SHRD_RX_MODE_RATE_0_1_B0, 0x12),
+	QMP_PHY_INIT_CFG(QSERDES_V6_LN_SHRD_RX_MODE_RATE_0_1_B1, 0x12),
+	QMP_PHY_INIT_CFG(QSERDES_V6_LN_SHRD_RX_MODE_RATE_0_1_B2, 0xdb),
+	QMP_PHY_INIT_CFG(QSERDES_V6_LN_SHRD_RX_MODE_RATE_0_1_B3, 0x9a),
+	QMP_PHY_INIT_CFG(QSERDES_V6_LN_SHRD_RX_MODE_RATE_0_1_B4, 0x38),
+	QMP_PHY_INIT_CFG(QSERDES_V6_LN_SHRD_RX_MODE_RATE_0_1_B5, 0xb6),
+	QMP_PHY_INIT_CFG(QSERDES_V6_LN_SHRD_RX_MODE_RATE_0_1_B6, 0x64),
+	QMP_PHY_INIT_CFG(QSERDES_V6_LN_SHRD_RX_MARG_COARSE_THRESH1_RATE210, 0x1f),
+	QMP_PHY_INIT_CFG(QSERDES_V6_LN_SHRD_RX_MARG_COARSE_THRESH1_RATE3, 0x1f),
+	QMP_PHY_INIT_CFG(QSERDES_V6_LN_SHRD_RX_MARG_COARSE_THRESH2_RATE210, 0x1f),
+	QMP_PHY_INIT_CFG(QSERDES_V6_LN_SHRD_RX_MARG_COARSE_THRESH2_RATE3, 0x1f),
+	QMP_PHY_INIT_CFG(QSERDES_V6_LN_SHRD_RX_MARG_COARSE_THRESH3_RATE210, 0x1f),
+	QMP_PHY_INIT_CFG(QSERDES_V6_LN_SHRD_RX_MARG_COARSE_THRESH3_RATE3, 0x1f),
+	QMP_PHY_INIT_CFG(QSERDES_V6_LN_SHRD_RX_MARG_COARSE_THRESH4_RATE3, 0x1f),
+	QMP_PHY_INIT_CFG(QSERDES_V6_LN_SHRD_RX_MARG_COARSE_THRESH5_RATE3, 0x1f),
+	QMP_PHY_INIT_CFG(QSERDES_V6_LN_SHRD_RX_MARG_COARSE_THRESH6_RATE3, 0x1f),
+};
+
+static const struct qmp_phy_init_tbl x1e80100_qmp_gen4x8_pcie_serdes_tbl[] = {
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_STEP_SIZE1_MODE1, 0x26),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_STEP_SIZE2_MODE1, 0x03),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_CP_CTRL_MODE1, 0x06),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_RCTRL_MODE1, 0x16),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_CCTRL_MODE1, 0x36),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_CORECLK_DIV_MODE1, 0x08),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP1_MODE1, 0x04),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP2_MODE1, 0x0d),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_DEC_START_MODE1, 0x68),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_DIV_FRAC_START1_MODE1, 0xab),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_DIV_FRAC_START2_MODE1, 0xaa),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_DIV_FRAC_START3_MODE1, 0x02),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_HSCLK_SEL_1, 0x12),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_STEP_SIZE1_MODE0, 0xf8),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_STEP_SIZE2_MODE0, 0x01),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_CP_CTRL_MODE0, 0x06),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_RCTRL_MODE0, 0x16),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_CCTRL_MODE0, 0x36),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_CORE_CLK_DIV_MODE0, 0x0a),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP1_MODE0, 0x04),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP2_MODE0, 0x0d),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_DEC_START_MODE0, 0x41),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_DIV_FRAC_START1_MODE0, 0xab),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_DIV_FRAC_START2_MODE0, 0xaa),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_DIV_FRAC_START3_MODE0, 0x01),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_HSCLK_HS_SWITCH_SEL_1, 0x00),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_BG_TIMER, 0x0a),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_EN_CENTER, 0x01),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_PER1, 0x62),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_PER2, 0x02),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_POST_DIV_MUX, 0x40),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_BIAS_EN_CLK_BUFLR_EN, 0x1c),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_CLK_ENABLE1, 0x90),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_SYS_CLK_CTRL, 0x82),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_IVCO, 0x0f),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_SYSCLK_EN_SEL, 0x08),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP_EN, 0x46),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP_CFG, 0x04),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_VCO_TUNE_MAP, 0x14),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_CLK_SELECT, 0x34),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_CORE_CLK_EN, 0x20),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_CMN_CONFIG_1, 0x06),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_CMN_MISC_1, 0x88),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_CMN_MODE, 0x14),
+	QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_VCO_DC_LEVEL_CTRL, 0x0f),
+};
+
+static const struct qmp_phy_init_tbl x1e80100_qmp_gen4x8_pcie_pcs_tbl[] = {
+	QMP_PHY_INIT_CFG(QPHY_V6_30_PCS_LOCK_DETECT_CONFIG2, 0x00),
+	QMP_PHY_INIT_CFG(QPHY_V6_30_PCS_G3S2_PRE_GAIN, 0x2e),
+	QMP_PHY_INIT_CFG(QPHY_V6_30_PCS_RX_SIGDET_LVL, 0x99),
+	QMP_PHY_INIT_CFG(QPHY_V6_30_PCS_ALIGN_DETECT_CONFIG7, 0x00),
+	QMP_PHY_INIT_CFG(QPHY_V6_30_PCS_EQ_CONFIG4, 0x00),
+	QMP_PHY_INIT_CFG(QPHY_V6_30_PCS_EQ_CONFIG5, 0x22),
+	QMP_PHY_INIT_CFG(QPHY_V6_30_PCS_TX_RX_CONFIG, 0x04),
+	QMP_PHY_INIT_CFG(QPHY_V6_30_PCS_TX_RX_CONFIG2, 0x02),
+};
+
+static const struct qmp_phy_init_tbl x1e80100_qmp_gen4x8_pcie_pcs_misc_tbl[] = {
+	QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_ENDPOINT_REFCLK_DRIVE, 0xc1),
+	QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_OSC_DTCT_ACTIONS, 0x00),
+	QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_EQ_CONFIG1, 0x16),
+	QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_G4_EQ_CONFIG5, 0x02),
+	QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_G4_PRE_GAIN, 0x2e),
+	QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_RX_MARGINING_CONFIG1, 0x03),
+	QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_RX_MARGINING_CONFIG3, 0x28),
+	QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_RX_MARGINING_CONFIG5, 0x18),
+	QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_G3_FOM_EQ_CONFIG5, 0x7a),
+	QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_G4_FOM_EQ_CONFIG5, 0x8a),
+	QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_G3_RXEQEVAL_TIME, 0x27),
+	QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_G4_RXEQEVAL_TIME, 0x27),
+	QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_TX_RX_CONFIG, 0xc0),
+	QMP_PHY_INIT_CFG(QPHY_PCIE_V6_30_PCS_POWER_STATE_CONFIG2, 0x1d),
+
+};
+
 static const struct qmp_phy_init_tbl sm8250_qmp_pcie_serdes_tbl[] = {
 	QMP_PHY_INIT_CFG(QSERDES_V4_COM_SYSCLK_EN_SEL, 0x08),
 	QMP_PHY_INIT_CFG(QSERDES_V4_COM_CLK_SELECT, 0x34),
@@ -2578,6 +2729,8 @@ struct qmp_pcie_offsets {
 	u16 rx;
 	u16 tx2;
 	u16 rx2;
+	u16 txz;
+	u16 rxz;
 	u16 ln_shrd;
 };
 
@@ -2588,6 +2741,10 @@ struct qmp_phy_cfg_tbls {
 	int tx_num;
 	const struct qmp_phy_init_tbl *rx;
 	int rx_num;
+	const struct qmp_phy_init_tbl *txz;
+	int txz_num;
+	const struct qmp_phy_init_tbl *rxz;
+	int rxz_num;
 	const struct qmp_phy_init_tbl *pcs;
 	int pcs_num;
 	const struct qmp_phy_init_tbl *pcs_misc;
@@ -2617,6 +2774,9 @@ struct qmp_phy_cfg {
 	const struct qmp_phy_init_tbl *serdes_4ln_tbl;
 	int serdes_4ln_num;
 
+	/* Set to true for programming all 8 lanes using txz/rxz registers */
+	bool lane_broadcasting;
+
 	/* resets to be requested */
 	const char * const *reset_list;
 	int num_resets;
@@ -2655,6 +2815,8 @@ struct qmp_pcie {
 	void __iomem *rx;
 	void __iomem *tx2;
 	void __iomem *rx2;
+	void __iomem *txz;
+	void __iomem *rxz;
 	void __iomem *ln_shrd;
 
 	void __iomem *port_b;
@@ -2700,7 +2862,7 @@ static inline void qphy_clrbits(void __iomem *base, u32 offset, u32 val)
 
 /* list of clocks required by phy */
 static const char * const qmp_pciephy_clk_l[] = {
-	"aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux",
+	"aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux", "clkref_en",
 };
 
 /* list of regulators */
@@ -2822,6 +2984,17 @@ static const struct qmp_pcie_offsets qmp_pcie_offsets_v6_20 = {
 	.ln_shrd	= 0x0e00,
 };
 
+static const struct qmp_pcie_offsets qmp_pcie_offsets_v6_30 = {
+	.serdes		= 0x8800,
+	.pcs		= 0x9000,
+	.pcs_misc	= 0x9800,
+	.tx		= 0x0000,
+	.rx		= 0x0200,
+	.txz		= 0xe000,
+	.rxz		= 0xe200,
+	.ln_shrd	= 0x8000,
+};
+
 static const struct qmp_phy_cfg ipq8074_pciephy_cfg = {
 	.lanes			= 1,
 
@@ -3665,6 +3838,40 @@ static const struct qmp_phy_cfg x1e80100_qmp_gen4x2_pciephy_cfg = {
 	.has_nocsr_reset	= true,
 };
 
+static const struct qmp_phy_cfg x1e80100_qmp_gen4x8_pciephy_cfg = {
+	.lanes = 8,
+
+	.offsets		= &qmp_pcie_offsets_v6_30,
+	.tbls = {
+		.serdes			= x1e80100_qmp_gen4x8_pcie_serdes_tbl,
+		.serdes_num		= ARRAY_SIZE(x1e80100_qmp_gen4x8_pcie_serdes_tbl),
+		.rx			= x1e80100_qmp_gen4x8_pcie_rx_tbl,
+		.rx_num			= ARRAY_SIZE(x1e80100_qmp_gen4x8_pcie_rx_tbl),
+		.pcs			= x1e80100_qmp_gen4x8_pcie_pcs_tbl,
+		.pcs_num		= ARRAY_SIZE(x1e80100_qmp_gen4x8_pcie_pcs_tbl),
+		.pcs_misc		= x1e80100_qmp_gen4x8_pcie_pcs_misc_tbl,
+		.pcs_misc_num		= ARRAY_SIZE(x1e80100_qmp_gen4x8_pcie_pcs_misc_tbl),
+		.ln_shrd		= x1e80100_qmp_gen4x8_pcie_ln_shrd_tbl,
+		.ln_shrd_num		= ARRAY_SIZE(x1e80100_qmp_gen4x8_pcie_ln_shrd_tbl),
+		.txz			= x1e80100_qmp_gen4x8_pcie_txz_tbl,
+		.txz_num		= ARRAY_SIZE(x1e80100_qmp_gen4x8_pcie_txz_tbl),
+		.rxz			= x1e80100_qmp_gen4x8_pcie_rxz_tbl,
+		.rxz_num		= ARRAY_SIZE(x1e80100_qmp_gen4x8_pcie_rxz_tbl),
+	},
+
+	.lane_broadcasting      = true,
+
+	.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 void qmp_pcie_init_port_b(struct qmp_pcie *qmp, const struct qmp_phy_cfg_tbls *tbls)
 {
 	const struct qmp_phy_cfg *cfg = qmp->cfg;
@@ -3700,6 +3907,11 @@ static void qmp_pcie_init_registers(struct qmp_pcie *qmp, const struct qmp_phy_c
 
 	qmp_configure(qmp->dev, serdes, tbls->serdes, tbls->serdes_num);
 
+	if (cfg->lane_broadcasting) {
+		qmp_configure(qmp->dev, qmp->txz, tbls->txz, tbls->txz_num);
+		qmp_configure(qmp->dev, qmp->rxz, tbls->rxz, tbls->rxz_num);
+	}
+
 	qmp_configure_lane(qmp->dev, tx, tbls->tx, tbls->tx_num, 1);
 	qmp_configure_lane(qmp->dev, rx, tbls->rx, tbls->rx_num, 1);
 
@@ -4242,6 +4454,11 @@ static int qmp_pcie_parse_dt(struct qmp_pcie *qmp)
 			return PTR_ERR(qmp->port_b);
 	}
 
+	if (cfg->lane_broadcasting) {
+		qmp->txz = base + offs->txz;
+		qmp->rxz = base + offs->rxz;
+	}
+
 	if (cfg->tbls.ln_shrd)
 		qmp->ln_shrd = base + offs->ln_shrd;
 
@@ -4424,6 +4641,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-gen4x8-pcie-phy",
+		.data = &x1e80100_qmp_gen4x8_pciephy_cfg,
 	},
 	{ },
 };
-- 
2.34.1

Re: [PATCH 3/8] phy: qcom: qmp: Add phy register and clk setting for x1e80100 PCIe3

[Konrad Dybcio]

On 27.08.2024 8:36 AM, Qiang Yu wrote:
> Currently driver supports only x4 lane based functionality using tx/rx and
> tx2/rx2 pair of register sets. To support 8 lane functionality with PCIe3,
> PCIe3 related QMP PHY provides additional programming which are available
> as txz and rxz based register set. Hence adds txz and rxz based registers
> usage and programming sequences. Phy register setting for txz and rxz will
> be applied to all 8 lanes. Some lanes may have different settings on
> several registers than txz/rxz, these registers should be programmed after
> txz/rxz programming sequences completing.
> 
> Besides, PCIe3 related QMP PHY also requires addtional clk, which is named
> as clkref_en. Hence, add this clk into qmp_pciephy_clk_l so that it can be
> easily parsed from devicetree during init.
> 
> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> ---

[...]

> +static const struct qmp_phy_init_tbl x1e80100_qmp_gen4x8_pcie_rx_tbl[] = {
> +	QMP_PHY_INIT_CFG_LANE(QSERDES_V6_20_RX_DFE_CTLE_POST_CAL_OFFSET, 0x3a, 1),

1 -> BIT(0)

[...]

> +	/* Set to true for programming all 8 lanes using txz/rxz registers */
> +	bool lane_broadcasting;

This is unnecessary because you call qmp_configure_lane conditionally,
but that function has a nullcheck built in

> +
>  	/* resets to be requested */
>  	const char * const *reset_list;
>  	int num_resets;
> @@ -2655,6 +2815,8 @@ struct qmp_pcie {
>  	void __iomem *rx;
>  	void __iomem *tx2;
>  	void __iomem *rx2;
> +	void __iomem *txz;
> +	void __iomem *rxz;
>  	void __iomem *ln_shrd;
>  
>  	void __iomem *port_b;
> @@ -2700,7 +2862,7 @@ static inline void qphy_clrbits(void __iomem *base, u32 offset, u32 val)
>  
>  /* list of clocks required by phy */
>  static const char * const qmp_pciephy_clk_l[] = {
> -	"aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux",
> +	"aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux", "clkref_en",

Why not just put in TCSR_PCIE_8L_CLKREF_EN as "ref"? It's downstream
of the XO anyway.

[...]

>  	const struct qmp_phy_cfg *cfg = qmp->cfg;
> @@ -3700,6 +3907,11 @@ static void qmp_pcie_init_registers(struct qmp_pcie *qmp, const struct qmp_phy_c
>  
>  	qmp_configure(qmp->dev, serdes, tbls->serdes, tbls->serdes_num);
>  
> +	if (cfg->lane_broadcasting) {

All these ifs can be unconditional

Konrad

Re: [PATCH 3/8] phy: qcom: qmp: Add phy register and clk setting for x1e80100 PCIe3

[Qiang Yu]

On 8/27/2024 6:33 PM, Konrad Dybcio wrote:
> On 27.08.2024 8:36 AM, Qiang Yu wrote:
>> Currently driver supports only x4 lane based functionality using tx/rx and
>> tx2/rx2 pair of register sets. To support 8 lane functionality with PCIe3,
>> PCIe3 related QMP PHY provides additional programming which are available
>> as txz and rxz based register set. Hence adds txz and rxz based registers
>> usage and programming sequences. Phy register setting for txz and rxz will
>> be applied to all 8 lanes. Some lanes may have different settings on
>> several registers than txz/rxz, these registers should be programmed after
>> txz/rxz programming sequences completing.
>>
>> Besides, PCIe3 related QMP PHY also requires addtional clk, which is named
>> as clkref_en. Hence, add this clk into qmp_pciephy_clk_l so that it can be
>> easily parsed from devicetree during init.
>>
>> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
>> ---
> [...]
>
>> +static const struct qmp_phy_init_tbl x1e80100_qmp_gen4x8_pcie_rx_tbl[] = {
>> +	QMP_PHY_INIT_CFG_LANE(QSERDES_V6_20_RX_DFE_CTLE_POST_CAL_OFFSET, 0x3a, 1),
> 1 -> BIT(0)
>
> [...]
>
>> +	/* Set to true for programming all 8 lanes using txz/rxz registers */
>> +	bool lane_broadcasting;
> This is unnecessary because you call qmp_configure_lane conditionally,
> but that function has a nullcheck built in
Yes, there is null pointer check in qmp_configure_lane, will remove
lane_broadcating check.
>> +
>>   	/* resets to be requested */
>>   	const char * const *reset_list;
>>   	int num_resets;
>> @@ -2655,6 +2815,8 @@ struct qmp_pcie {
>>   	void __iomem *rx;
>>   	void __iomem *tx2;
>>   	void __iomem *rx2;
>> +	void __iomem *txz;
>> +	void __iomem *rxz;
>>   	void __iomem *ln_shrd;
>>   
>>   	void __iomem *port_b;
>> @@ -2700,7 +2862,7 @@ static inline void qphy_clrbits(void __iomem *base, u32 offset, u32 val)
>>   
>>   /* list of clocks required by phy */
>>   static const char * const qmp_pciephy_clk_l[] = {
>> -	"aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux",
>> +	"aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux", "clkref_en",
> Why not just put in TCSR_PCIE_8L_CLKREF_EN as "ref"? It's downstream
> of the XO anyway.
Yes, TCSR_PCIE_8L_CLKREF_EN is source from XO, will update patch as
your comments.

Thanks,
Qiang
>
> [...]
>
>>   	const struct qmp_phy_cfg *cfg = qmp->cfg;
>> @@ -3700,6 +3907,11 @@ static void qmp_pcie_init_registers(struct qmp_pcie *qmp, const struct qmp_phy_c
>>   
>>   	qmp_configure(qmp->dev, serdes, tbls->serdes, tbls->serdes_num);
>>   
>> +	if (cfg->lane_broadcasting) {
> All these ifs can be unconditional
>
> Konrad

Re: [PATCH 3/8] phy: qcom: qmp: Add phy register and clk setting for x1e80100 PCIe3

[Krzysztof Kozlowski]

On Mon, Aug 26, 2024 at 11:36:26PM -0700, Qiang Yu wrote:
>  	if (cfg->tbls.ln_shrd)
>  		qmp->ln_shrd = base + offs->ln_shrd;
>  
> @@ -4424,6 +4641,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-gen4x8-pcie-phy",

Undocumented compatible or your patch order is wrong.

> +		.data = &x1e80100_qmp_gen4x8_pciephy_cfg,
>  	},

Best regards,
Krzysztof

Re: [PATCH 3/8] phy: qcom: qmp: Add phy register and clk setting for x1e80100 PCIe3

[Qiang Yu]

On 8/27/2024 7:38 PM, Krzysztof Kozlowski wrote:
> On Mon, Aug 26, 2024 at 11:36:26PM -0700, Qiang Yu wrote:
>>   	if (cfg->tbls.ln_shrd)
>>   		qmp->ln_shrd = base + offs->ln_shrd;
>>   
>> @@ -4424,6 +4641,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-gen4x8-pcie-phy",
> Undocumented compatible or your patch order is wrong.
OK, will put the yaml patch in front of this patch

Thanks,
Qiang
>
>> +		.data = &x1e80100_qmp_gen4x8_pciephy_cfg,
>>   	},
> Best regards,
> Krzysztof
>

[PATCH 4/8] arm64: dts: qcom: x1e80100: Add support for PCIe3 on x1e80100

[Qiang Yu]

Describe PCIe3 controller and PHY. Also add required system resources like
regulators, clocks, interrupts and registers configuration for PCIe3.

Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
---
 arch/arm64/boot/dts/qcom/x1e80100.dtsi | 205 ++++++++++++++++++++++++-
 1 file changed, 204 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
index 74b694e74705..55b81e7de1c7 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
@@ -744,7 +744,7 @@ gcc: clock-controller@100000 {
 
 			clocks = <&bi_tcxo_div2>,
 				 <&sleep_clk>,
-				 <0>,
+				 <&pcie3_phy>,
 				 <&pcie4_phy>,
 				 <&pcie5_phy>,
 				 <&pcie6a_phy>,
@@ -2879,6 +2879,209 @@ mmss_noc: interconnect@1780000 {
 			#interconnect-cells = <2>;
 		};
 
+		pcie3: pci@1bd0000 {
+			device_type = "pci";
+			compatible = "qcom,pcie-x1e80100";
+			reg = <0 0x01bd0000 0 0x3000>,
+			      <0 0x78000000 0 0xf1d>,
+			      <0 0x78000f40 0 0xa8>,
+			      <0 0x78001000 0 0x1000>,
+			      <0 0x78100000 0 0x100000>;
+			reg-names = "parf",
+				    "dbi",
+				    "elbi",
+				    "atu",
+				    "config";
+			#address-cells = <3>;
+			#size-cells = <2>;
+			ranges = <0x01000000 0 0x00000000 0 0x78200000 0 0x100000>,
+				 <0x02000000 0 0x78300000 0 0x78300000 0 0x3d00000>;
+			bus-range = <0 0xff>;
+
+			dma-coherent;
+
+			linux,pci-domain = <3>;
+			num-lanes = <8>;
+
+			interrupts = <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 769 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 836 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 671 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 218 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 219 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "msi0",
+					  "msi1",
+					  "msi2",
+					  "msi3",
+					  "msi4",
+					  "msi5",
+					  "msi6",
+					  "msi7";
+
+			#interrupt-cells = <1>;
+			interrupt-map-mask = <0 0 0 0x7>;
+			interrupt-map = <0 0 0 1 &intc 0 0 0 220 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 2 &intc 0 0 0 221 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 3 &intc 0 0 0 237 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 4 &intc 0 0 0 238 IRQ_TYPE_LEVEL_HIGH>;
+
+			clocks = <&gcc GCC_PCIE_3_PIPE_CLK_SRC>,
+				 <&gcc GCC_PCIE_3_AUX_CLK>,
+				 <&gcc GCC_PCIE_3_CFG_AHB_CLK>,
+				 <&gcc GCC_PCIE_3_MSTR_AXI_CLK>,
+				 <&gcc GCC_PCIE_3_SLV_AXI_CLK>,
+				 <&gcc GCC_PCIE_3_SLV_Q2A_AXI_CLK>,
+				 <&gcc GCC_CFG_NOC_PCIE_ANOC_SOUTH_AHB_CLK>,
+				 <&gcc GCC_CNOC_PCIE_NORTH_SF_AXI_CLK>;
+			clock-names = "pipe_clk_src",
+				      "aux",
+				      "cfg",
+				      "bus_master",
+				      "bus_slave",
+				      "slave_q2a",
+				      "noc_aggr",
+				      "cnoc_sf_axi";
+
+			assigned-clocks = <&gcc GCC_PCIE_3_AUX_CLK>;
+			assigned-clock-rates = <19200000>;
+
+			interconnects = <&pcie_south_anoc MASTER_PCIE_3 QCOM_ICC_TAG_ALWAYS
+					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
+					<&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
+					 &cnoc_main SLAVE_PCIE_3 QCOM_ICC_TAG_ALWAYS>;
+			interconnect-names = "pcie-mem",
+					     "cpu-pcie";
+
+			resets = <&gcc GCC_PCIE_3_BCR>,
+				 <&gcc GCC_PCIE_3_LINK_DOWN_BCR>;
+			reset-names = "pci",
+				      "link_down";
+
+			power-domains = <&gcc GCC_PCIE_3_GDSC>;
+
+			phys = <&pcie3_phy>;
+			phy-names = "pciephy";
+
+			operating-points-v2 = <&pcie3_opp_table>;
+
+			status = "disabled";
+
+			pcie3_opp_table: opp-table {
+				compatible = "operating-points-v2";
+
+				/* GEN 1 x1 */
+				opp-2500000 {
+					opp-hz = /bits/ 64 <2500000>;
+					required-opps = <&rpmhpd_opp_low_svs>;
+					opp-peak-kBps = <250000 1>;
+				};
+
+				/* GEN 1 x2 and GEN 2 x1 */
+				opp-5000000 {
+					opp-hz = /bits/ 64 <5000000>;
+					required-opps = <&rpmhpd_opp_low_svs>;
+					opp-peak-kBps = <500000 1>;
+				};
+
+				/* GEN 1 x4 and GEN 2 x2*/
+				opp-10000000 {
+					opp-hz = /bits/ 64 <10000000>;
+					required-opps = <&rpmhpd_opp_low_svs>;
+					opp-peak-kBps = <1000000 1>;
+				};
+
+				/* GEN 1 x8 and GEN 2 X4 */
+				opp-20000000 {
+					opp-hz = /bits/ 64 <20000000>;
+					required-opps = <&rpmhpd_opp_low_svs>;
+					opp-peak-kBps = <2000000 1>;
+				};
+
+				/* GEN 2 x8 */
+				opp-40000000 {
+					opp-hz = /bits/ 64 <40000000>;
+					required-opps = <&rpmhpd_opp_low_svs>;
+					opp-peak-kBps = <4000000 1>;
+				};
+
+				/* GEN 3 x1 */
+				opp-8000000 {
+					opp-hz = /bits/ 64 <8000000>;
+					required-opps = <&rpmhpd_opp_svs>;
+					opp-peak-kBps = <984500 1>;
+				};
+
+				/* GEN 3 x2 and GEN 4 x1 */
+				opp-16000000 {
+					opp-hz = /bits/ 64 <16000000>;
+					required-opps = <&rpmhpd_opp_svs>;
+					opp-peak-kBps = <1969000 1>;
+				};
+
+				/* GEN 3 x4 and GEN 4 x2 */
+				opp-32000000 {
+					opp-hz = /bits/ 64 <32000000>;
+					required-opps = <&rpmhpd_opp_svs>;
+					opp-peak-kBps = <3938000 1>;
+				};
+
+				/* GEN 3 x8 and GEN 4 x4 */
+				opp-64000000 {
+					opp-hz = /bits/ 64 <64000000>;
+					required-opps = <&rpmhpd_opp_svs>;
+					opp-peak-kBps = <7876000 1>;
+				};
+
+				/* GEN 4 x8 */
+				opp-128000000 {
+					opp-hz = /bits/ 64 <128000000>;
+					required-opps = <&rpmhpd_opp_svs>;
+					opp-peak-kBps = <15753000 1>;
+				};
+			};
+		};
+
+		pcie3_phy: phy@1be0000 {
+			compatible = "qcom,x1e80100-qmp-gen4x8-pcie-phy";
+			reg = <0 0x01be0000 0 0x10000>;
+
+			clocks = <&gcc GCC_PCIE_3_AUX_CLK>,
+				 <&gcc GCC_PCIE_3_CFG_AHB_CLK>,
+				 <&rpmhcc RPMH_CXO_CLK>,
+				 <&gcc GCC_PCIE_3_PHY_RCHNG_CLK>,
+				 <&gcc GCC_PCIE_3_PHY_AUX_CLK>,
+				 <&gcc GCC_PCIE_3_PIPE_CLK>,
+				 <&gcc GCC_PCIE_3_PIPEDIV2_CLK>,
+				 <&tcsr TCSR_PCIE_8L_CLKREF_EN>;
+			clock-names = "aux",
+				      "cfg_ahb",
+				      "ref",
+				      "rchng",
+				      "phy_aux",
+				      "pipe",
+				      "pipediv2",
+				      "clkref_en";
+
+			resets = <&gcc GCC_PCIE_3_PHY_BCR>,
+				 <&gcc GCC_PCIE_3_NOCSR_COM_PHY_BCR>;
+			reset-names = "phy",
+				      "phy_nocsr";
+
+			assigned-clocks = <&gcc GCC_PCIE_3_PHY_RCHNG_CLK>;
+			assigned-clock-rates = <100000000>;
+
+			power-domains = <&gcc GCC_PCIE_3_PHY_GDSC>;
+
+			#clock-cells = <0>;
+			clock-output-names = "pcie3_pipe_clk";
+
+			#phy-cells = <0>;
+
+			status = "disabled";
+		};
+
 		pcie6a: pci@1bf8000 {
 			device_type = "pci";
 			compatible = "qcom,pcie-x1e80100";
-- 
2.34.1

Re: [PATCH 4/8] arm64: dts: qcom: x1e80100: Add support for PCIe3 on x1e80100

[Konrad Dybcio]

On 27.08.2024 8:36 AM, Qiang Yu wrote:
> Describe PCIe3 controller and PHY. Also add required system resources like
> regulators, clocks, interrupts and registers configuration for PCIe3.
> 
> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/x1e80100.dtsi | 205 ++++++++++++++++++++++++-
>  1 file changed, 204 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> index 74b694e74705..55b81e7de1c7 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> @@ -744,7 +744,7 @@ gcc: clock-controller@100000 {
>  
>  			clocks = <&bi_tcxo_div2>,
>  				 <&sleep_clk>,
> -				 <0>,
> +				 <&pcie3_phy>,
>  				 <&pcie4_phy>,
>  				 <&pcie5_phy>,
>  				 <&pcie6a_phy>,
> @@ -2879,6 +2879,209 @@ mmss_noc: interconnect@1780000 {
>  			#interconnect-cells = <2>;
>  		};
>  
> +		pcie3: pci@1bd0000 {
> +			device_type = "pci";
> +			compatible = "qcom,pcie-x1e80100";
> +			reg = <0 0x01bd0000 0 0x3000>,
> +			      <0 0x78000000 0 0xf1d>,
> +			      <0 0x78000f40 0 0xa8>,
> +			      <0 0x78001000 0 0x1000>,
> +			      <0 0x78100000 0 0x100000>;
> +			reg-names = "parf",
> +				    "dbi",
> +				    "elbi",
> +				    "atu",
> +				    "config";

There's a "mhi" region at 0x01bd3000, 0x1000-wide too, please add it

> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +			ranges = <0x01000000 0 0x00000000 0 0x78200000 0 0x100000>,
> +				 <0x02000000 0 0x78300000 0 0x78300000 0 0x3d00000>;

There's 64bit BAR space as well:

<0x03000000 0x7 0x40000000 0x7 0x40000000 0x0 0x40000000>;

> +			bus-range = <0 0xff>;

0x00 please

> +
> +			dma-coherent;
> +
> +			linux,pci-domain = <3>;
> +			num-lanes = <8>;
> +
> +			interrupts = <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 769 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 836 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 671 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 218 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 219 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "msi0",
> +					  "msi1",
> +					  "msi2",
> +					  "msi3",
> +					  "msi4",
> +					  "msi5",
> +					  "msi6",
> +					  "msi7";
> +
> +			#interrupt-cells = <1>;
> +			interrupt-map-mask = <0 0 0 0x7>;
> +			interrupt-map = <0 0 0 1 &intc 0 0 0 220 IRQ_TYPE_LEVEL_HIGH>,
> +					<0 0 0 2 &intc 0 0 0 221 IRQ_TYPE_LEVEL_HIGH>,
> +					<0 0 0 3 &intc 0 0 0 237 IRQ_TYPE_LEVEL_HIGH>,
> +					<0 0 0 4 &intc 0 0 0 238 IRQ_TYPE_LEVEL_HIGH>;
> +
> +			clocks = <&gcc GCC_PCIE_3_PIPE_CLK_SRC>,

We don't toggle source clocks from dt, this is upstream of the pipe
div clocks and is taken care of by the common clock framework,
please drop.

> +				 <&gcc GCC_PCIE_3_AUX_CLK>,
> +				 <&gcc GCC_PCIE_3_CFG_AHB_CLK>,
> +				 <&gcc GCC_PCIE_3_MSTR_AXI_CLK>,
> +				 <&gcc GCC_PCIE_3_SLV_AXI_CLK>,
> +				 <&gcc GCC_PCIE_3_SLV_Q2A_AXI_CLK>,
> +				 <&gcc GCC_CFG_NOC_PCIE_ANOC_SOUTH_AHB_CLK>,

GCC_CFG_NOC_PCIE_ANOC_NORTH_AHB_CLK

> +				 <&gcc GCC_CNOC_PCIE_NORTH_SF_AXI_CLK>;
> +			clock-names = "pipe_clk_src",
> +				      "aux",
> +				      "cfg",
> +				      "bus_master",
> +				      "bus_slave",
> +				      "slave_q2a",
> +				      "noc_aggr",
> +				      "cnoc_sf_axi";
> +
> +			assigned-clocks = <&gcc GCC_PCIE_3_AUX_CLK>;
> +			assigned-clock-rates = <19200000>;
> +
> +			interconnects = <&pcie_south_anoc MASTER_PCIE_3 QCOM_ICC_TAG_ALWAYS
> +					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
> +					<&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
> +					 &cnoc_main SLAVE_PCIE_3 QCOM_ICC_TAG_ALWAYS>;
> +			interconnect-names = "pcie-mem",
> +					     "cpu-pcie";
> +
> +			resets = <&gcc GCC_PCIE_3_BCR>,
> +				 <&gcc GCC_PCIE_3_LINK_DOWN_BCR>;
> +			reset-names = "pci",
> +				      "link_down";
> +
> +			power-domains = <&gcc GCC_PCIE_3_GDSC>;
> +
> +			phys = <&pcie3_phy>;
> +			phy-names = "pciephy";
> +
> +			operating-points-v2 = <&pcie3_opp_table>;
> +
> +			status = "disabled";
> +
> +			pcie3_opp_table: opp-table {
> +				compatible = "operating-points-v2";
> +
> +				/* GEN 1 x1 */
> +				opp-2500000 {
> +					opp-hz = /bits/ 64 <2500000>;
> +					required-opps = <&rpmhpd_opp_low_svs>;
> +					opp-peak-kBps = <250000 1>;
> +				};
> +
> +				/* GEN 1 x2 and GEN 2 x1 */
> +				opp-5000000 {
> +					opp-hz = /bits/ 64 <5000000>;
> +					required-opps = <&rpmhpd_opp_low_svs>;
> +					opp-peak-kBps = <500000 1>;
> +				};
> +
> +				/* GEN 1 x4 and GEN 2 x2*/

Missing ' '

> +				opp-10000000 {
> +					opp-hz = /bits/ 64 <10000000>;
> +					required-opps = <&rpmhpd_opp_low_svs>;
> +					opp-peak-kBps = <1000000 1>;
> +				};
> +
> +				/* GEN 1 x8 and GEN 2 X4 */

Inconsistent capitalization, please use lowercase 'x'

[...]

> +		pcie3_phy: phy@1be0000 {
> +			compatible = "qcom,x1e80100-qmp-gen4x8-pcie-phy";
> +			reg = <0 0x01be0000 0 0x10000>;
> +
> +			clocks = <&gcc GCC_PCIE_3_AUX_CLK>,

This clock doesn't belong here, the PHY is clocked by PHY_AUX

> +				 <&gcc GCC_PCIE_3_CFG_AHB_CLK>,
> +				 <&rpmhcc RPMH_CXO_CLK>,

This is unnecessary as commented before

> +				 <&gcc GCC_PCIE_3_PHY_RCHNG_CLK>,
> +				 <&gcc GCC_PCIE_3_PHY_AUX_CLK>,
> +				 <&gcc GCC_PCIE_3_PIPE_CLK>,
> +				 <&gcc GCC_PCIE_3_PIPEDIV2_CLK>,
> +				 <&tcsr TCSR_PCIE_8L_CLKREF_EN>;

This should be the 'ref' here

Konrad

Re: [PATCH 4/8] arm64: dts: qcom: x1e80100: Add support for PCIe3 on x1e80100

[Qiang Yu]

On 8/27/2024 6:42 PM, Konrad Dybcio wrote:
> On 27.08.2024 8:36 AM, Qiang Yu wrote:
>> Describe PCIe3 controller and PHY. Also add required system resources like
>> regulators, clocks, interrupts and registers configuration for PCIe3.
>>
>> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/x1e80100.dtsi | 205 ++++++++++++++++++++++++-
>>   1 file changed, 204 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>> index 74b694e74705..55b81e7de1c7 100644
>> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>> @@ -744,7 +744,7 @@ gcc: clock-controller@100000 {
>>   
>>   			clocks = <&bi_tcxo_div2>,
>>   				 <&sleep_clk>,
>> -				 <0>,
>> +				 <&pcie3_phy>,
>>   				 <&pcie4_phy>,
>>   				 <&pcie5_phy>,
>>   				 <&pcie6a_phy>,
>> @@ -2879,6 +2879,209 @@ mmss_noc: interconnect@1780000 {
>>   			#interconnect-cells = <2>;
>>   		};
>>   
>> +		pcie3: pci@1bd0000 {
>> +			device_type = "pci";
>> +			compatible = "qcom,pcie-x1e80100";
>> +			reg = <0 0x01bd0000 0 0x3000>,
>> +			      <0 0x78000000 0 0xf1d>,
>> +			      <0 0x78000f40 0 0xa8>,
>> +			      <0 0x78001000 0 0x1000>,
>> +			      <0 0x78100000 0 0x100000>;
>> +			reg-names = "parf",
>> +				    "dbi",
>> +				    "elbi",
>> +				    "atu",
>> +				    "config";
> There's a "mhi" region at 0x01bd3000, 0x1000-wide too, please add it
>
>> +			#address-cells = <3>;
>> +			#size-cells = <2>;
>> +			ranges = <0x01000000 0 0x00000000 0 0x78200000 0 0x100000>,
>> +				 <0x02000000 0 0x78300000 0 0x78300000 0 0x3d00000>;
> There's 64bit BAR space as well:
>
> <0x03000000 0x7 0x40000000 0x7 0x40000000 0x0 0x40000000>;
>
>> +			bus-range = <0 0xff>;
> 0x00 please
>
>> +
>> +			dma-coherent;
>> +
>> +			linux,pci-domain = <3>;
>> +			num-lanes = <8>;
>> +
>> +			interrupts = <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>,
>> +				     <GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>,
>> +				     <GIC_SPI 769 IRQ_TYPE_LEVEL_HIGH>,
>> +				     <GIC_SPI 836 IRQ_TYPE_LEVEL_HIGH>,
>> +				     <GIC_SPI 671 IRQ_TYPE_LEVEL_HIGH>,
>> +				     <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>,
>> +				     <GIC_SPI 218 IRQ_TYPE_LEVEL_HIGH>,
>> +				     <GIC_SPI 219 IRQ_TYPE_LEVEL_HIGH>;
>> +			interrupt-names = "msi0",
>> +					  "msi1",
>> +					  "msi2",
>> +					  "msi3",
>> +					  "msi4",
>> +					  "msi5",
>> +					  "msi6",
>> +					  "msi7";
>> +
>> +			#interrupt-cells = <1>;
>> +			interrupt-map-mask = <0 0 0 0x7>;
>> +			interrupt-map = <0 0 0 1 &intc 0 0 0 220 IRQ_TYPE_LEVEL_HIGH>,
>> +					<0 0 0 2 &intc 0 0 0 221 IRQ_TYPE_LEVEL_HIGH>,
>> +					<0 0 0 3 &intc 0 0 0 237 IRQ_TYPE_LEVEL_HIGH>,
>> +					<0 0 0 4 &intc 0 0 0 238 IRQ_TYPE_LEVEL_HIGH>;
>> +
>> +			clocks = <&gcc GCC_PCIE_3_PIPE_CLK_SRC>,
> We don't toggle source clocks from dt, this is upstream of the pipe
> div clocks and is taken care of by the common clock framework,
> please drop.
GCC_PCIE_3_PIPE_CLK_SRC is a clk mux. The enable and disable callback
provided in clk driver is used to switch between pipe_clk and XO,
respectively. If we drop GCC_PCIE_3_PIPE_CLK_SRC here, that means
the mux will be XO until pipediv2 clk is enabled. I need to do some
experiment to check this. Will update in thread.

Thanks,
Qiang
>> +				 <&gcc GCC_PCIE_3_AUX_CLK>,
>> +				 <&gcc GCC_PCIE_3_CFG_AHB_CLK>,
>> +				 <&gcc GCC_PCIE_3_MSTR_AXI_CLK>,
>> +				 <&gcc GCC_PCIE_3_SLV_AXI_CLK>,
>> +				 <&gcc GCC_PCIE_3_SLV_Q2A_AXI_CLK>,
>> +				 <&gcc GCC_CFG_NOC_PCIE_ANOC_SOUTH_AHB_CLK>,
> GCC_CFG_NOC_PCIE_ANOC_NORTH_AHB_CLK
>
>> +				 <&gcc GCC_CNOC_PCIE_NORTH_SF_AXI_CLK>;
>> +			clock-names = "pipe_clk_src",
>> +				      "aux",
>> +				      "cfg",
>> +				      "bus_master",
>> +				      "bus_slave",
>> +				      "slave_q2a",
>> +				      "noc_aggr",
>> +				      "cnoc_sf_axi";
>> +
>> +			assigned-clocks = <&gcc GCC_PCIE_3_AUX_CLK>;
>> +			assigned-clock-rates = <19200000>;
>> +
>> +			interconnects = <&pcie_south_anoc MASTER_PCIE_3 QCOM_ICC_TAG_ALWAYS
>> +					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
>> +					<&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
>> +					 &cnoc_main SLAVE_PCIE_3 QCOM_ICC_TAG_ALWAYS>;
>> +			interconnect-names = "pcie-mem",
>> +					     "cpu-pcie";
>> +
>> +			resets = <&gcc GCC_PCIE_3_BCR>,
>> +				 <&gcc GCC_PCIE_3_LINK_DOWN_BCR>;
>> +			reset-names = "pci",
>> +				      "link_down";
>> +
>> +			power-domains = <&gcc GCC_PCIE_3_GDSC>;
>> +
>> +			phys = <&pcie3_phy>;
>> +			phy-names = "pciephy";
>> +
>> +			operating-points-v2 = <&pcie3_opp_table>;
>> +
>> +			status = "disabled";
>> +
>> +			pcie3_opp_table: opp-table {
>> +				compatible = "operating-points-v2";
>> +
>> +				/* GEN 1 x1 */
>> +				opp-2500000 {
>> +					opp-hz = /bits/ 64 <2500000>;
>> +					required-opps = <&rpmhpd_opp_low_svs>;
>> +					opp-peak-kBps = <250000 1>;
>> +				};
>> +
>> +				/* GEN 1 x2 and GEN 2 x1 */
>> +				opp-5000000 {
>> +					opp-hz = /bits/ 64 <5000000>;
>> +					required-opps = <&rpmhpd_opp_low_svs>;
>> +					opp-peak-kBps = <500000 1>;
>> +				};
>> +
>> +				/* GEN 1 x4 and GEN 2 x2*/
> Missing ' '
>
>> +				opp-10000000 {
>> +					opp-hz = /bits/ 64 <10000000>;
>> +					required-opps = <&rpmhpd_opp_low_svs>;
>> +					opp-peak-kBps = <1000000 1>;
>> +				};
>> +
>> +				/* GEN 1 x8 and GEN 2 X4 */
> Inconsistent capitalization, please use lowercase 'x'
>
> [...]
>
>> +		pcie3_phy: phy@1be0000 {
>> +			compatible = "qcom,x1e80100-qmp-gen4x8-pcie-phy";
>> +			reg = <0 0x01be0000 0 0x10000>;
>> +
>> +			clocks = <&gcc GCC_PCIE_3_AUX_CLK>,
> This clock doesn't belong here, the PHY is clocked by PHY_AUX
>
>> +				 <&gcc GCC_PCIE_3_CFG_AHB_CLK>,
>> +				 <&rpmhcc RPMH_CXO_CLK>,
> This is unnecessary as commented before
>
>> +				 <&gcc GCC_PCIE_3_PHY_RCHNG_CLK>,
>> +				 <&gcc GCC_PCIE_3_PHY_AUX_CLK>,
>> +				 <&gcc GCC_PCIE_3_PIPE_CLK>,
>> +				 <&gcc GCC_PCIE_3_PIPEDIV2_CLK>,
>> +				 <&tcsr TCSR_PCIE_8L_CLKREF_EN>;
> This should be the 'ref' here
>
> Konrad

Re: [PATCH 4/8] arm64: dts: qcom: x1e80100: Add support for PCIe3 on x1e80100

[Qiang Yu]

On 8/28/2024 9:36 PM, Qiang Yu wrote:
>
> On 8/27/2024 6:42 PM, Konrad Dybcio wrote:
>> On 27.08.2024 8:36 AM, Qiang Yu wrote:
>>> Describe PCIe3 controller and PHY. Also add required system 
>>> resources like
>>> regulators, clocks, interrupts and registers configuration for PCIe3.
>>>
>>> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
>>> ---
>>>   arch/arm64/boot/dts/qcom/x1e80100.dtsi | 205 
>>> ++++++++++++++++++++++++-
>>>   1 file changed, 204 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi 
>>> b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>>> index 74b694e74705..55b81e7de1c7 100644
>>> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>>> @@ -744,7 +744,7 @@ gcc: clock-controller@100000 {
>>>                 clocks = <&bi_tcxo_div2>,
>>>                    <&sleep_clk>,
>>> -                 <0>,
>>> +                 <&pcie3_phy>,
>>>                    <&pcie4_phy>,
>>>                    <&pcie5_phy>,
>>>                    <&pcie6a_phy>,
>>> @@ -2879,6 +2879,209 @@ mmss_noc: interconnect@1780000 {
>>>               #interconnect-cells = <2>;
>>>           };
>>>   +        pcie3: pci@1bd0000 {
>>> +            device_type = "pci";
>>> +            compatible = "qcom,pcie-x1e80100";
>>> +            reg = <0 0x01bd0000 0 0x3000>,
>>> +                  <0 0x78000000 0 0xf1d>,
>>> +                  <0 0x78000f40 0 0xa8>,
>>> +                  <0 0x78001000 0 0x1000>,
>>> +                  <0 0x78100000 0 0x100000>;
>>> +            reg-names = "parf",
>>> +                    "dbi",
>>> +                    "elbi",
>>> +                    "atu",
>>> +                    "config";
>> There's a "mhi" region at 0x01bd3000, 0x1000-wide too, please add it
>>
>>> +            #address-cells = <3>;
>>> +            #size-cells = <2>;
>>> +            ranges = <0x01000000 0 0x00000000 0 0x78200000 0 
>>> 0x100000>,
>>> +                 <0x02000000 0 0x78300000 0 0x78300000 0 0x3d00000>;
>> There's 64bit BAR space as well:
>>
>> <0x03000000 0x7 0x40000000 0x7 0x40000000 0x0 0x40000000>;
>>
>>> +            bus-range = <0 0xff>;
>> 0x00 please
>>
>>> +
>>> +            dma-coherent;
>>> +
>>> +            linux,pci-domain = <3>;
>>> +            num-lanes = <8>;
>>> +
>>> +            interrupts = <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>,
>>> +                     <GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>,
>>> +                     <GIC_SPI 769 IRQ_TYPE_LEVEL_HIGH>,
>>> +                     <GIC_SPI 836 IRQ_TYPE_LEVEL_HIGH>,
>>> +                     <GIC_SPI 671 IRQ_TYPE_LEVEL_HIGH>,
>>> +                     <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>,
>>> +                     <GIC_SPI 218 IRQ_TYPE_LEVEL_HIGH>,
>>> +                     <GIC_SPI 219 IRQ_TYPE_LEVEL_HIGH>;
>>> +            interrupt-names = "msi0",
>>> +                      "msi1",
>>> +                      "msi2",
>>> +                      "msi3",
>>> +                      "msi4",
>>> +                      "msi5",
>>> +                      "msi6",
>>> +                      "msi7";
>>> +
>>> +            #interrupt-cells = <1>;
>>> +            interrupt-map-mask = <0 0 0 0x7>;
>>> +            interrupt-map = <0 0 0 1 &intc 0 0 0 220 
>>> IRQ_TYPE_LEVEL_HIGH>,
>>> +                    <0 0 0 2 &intc 0 0 0 221 IRQ_TYPE_LEVEL_HIGH>,
>>> +                    <0 0 0 3 &intc 0 0 0 237 IRQ_TYPE_LEVEL_HIGH>,
>>> +                    <0 0 0 4 &intc 0 0 0 238 IRQ_TYPE_LEVEL_HIGH>;
>>> +
>>> +            clocks = <&gcc GCC_PCIE_3_PIPE_CLK_SRC>,
>> We don't toggle source clocks from dt, this is upstream of the pipe
>> div clocks and is taken care of by the common clock framework,
>> please drop.
> GCC_PCIE_3_PIPE_CLK_SRC is a clk mux. The enable and disable callback
> provided in clk driver is used to switch between pipe_clk and XO,
> respectively. If we drop GCC_PCIE_3_PIPE_CLK_SRC here, that means
> the mux will be XO until pipediv2 clk is enabled. I need to do some
> experiment to check this. Will update in thread.
>
> Thanks,
> Qiang
After removing GCC_PCIE_3_PIPE_CLK_SRC, I tested it and link was up.

Thanks,
Qiang
>>> +                 <&gcc GCC_PCIE_3_AUX_CLK>,
>>> +                 <&gcc GCC_PCIE_3_CFG_AHB_CLK>,
>>> +                 <&gcc GCC_PCIE_3_MSTR_AXI_CLK>,
>>> +                 <&gcc GCC_PCIE_3_SLV_AXI_CLK>,
>>> +                 <&gcc GCC_PCIE_3_SLV_Q2A_AXI_CLK>,
>>> +                 <&gcc GCC_CFG_NOC_PCIE_ANOC_SOUTH_AHB_CLK>,
>> GCC_CFG_NOC_PCIE_ANOC_NORTH_AHB_CLK
>>
>>> +                 <&gcc GCC_CNOC_PCIE_NORTH_SF_AXI_CLK>;
>>> +            clock-names = "pipe_clk_src",
>>> +                      "aux",
>>> +                      "cfg",
>>> +                      "bus_master",
>>> +                      "bus_slave",
>>> +                      "slave_q2a",
>>> +                      "noc_aggr",
>>> +                      "cnoc_sf_axi";
>>> +
>>> +            assigned-clocks = <&gcc GCC_PCIE_3_AUX_CLK>;
>>> +            assigned-clock-rates = <19200000>;
>>> +
>>> +            interconnects = <&pcie_south_anoc MASTER_PCIE_3 
>>> QCOM_ICC_TAG_ALWAYS
>>> +                     &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
>>> +                    <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
>>> +                     &cnoc_main SLAVE_PCIE_3 QCOM_ICC_TAG_ALWAYS>;
>>> +            interconnect-names = "pcie-mem",
>>> +                         "cpu-pcie";
>>> +
>>> +            resets = <&gcc GCC_PCIE_3_BCR>,
>>> +                 <&gcc GCC_PCIE_3_LINK_DOWN_BCR>;
>>> +            reset-names = "pci",
>>> +                      "link_down";
>>> +
>>> +            power-domains = <&gcc GCC_PCIE_3_GDSC>;
>>> +
>>> +            phys = <&pcie3_phy>;
>>> +            phy-names = "pciephy";
>>> +
>>> +            operating-points-v2 = <&pcie3_opp_table>;
>>> +
>>> +            status = "disabled";
>>> +
>>> +            pcie3_opp_table: opp-table {
>>> +                compatible = "operating-points-v2";
>>> +
>>> +                /* GEN 1 x1 */
>>> +                opp-2500000 {
>>> +                    opp-hz = /bits/ 64 <2500000>;
>>> +                    required-opps = <&rpmhpd_opp_low_svs>;
>>> +                    opp-peak-kBps = <250000 1>;
>>> +                };
>>> +
>>> +                /* GEN 1 x2 and GEN 2 x1 */
>>> +                opp-5000000 {
>>> +                    opp-hz = /bits/ 64 <5000000>;
>>> +                    required-opps = <&rpmhpd_opp_low_svs>;
>>> +                    opp-peak-kBps = <500000 1>;
>>> +                };
>>> +
>>> +                /* GEN 1 x4 and GEN 2 x2*/
>> Missing ' '
>>
>>> +                opp-10000000 {
>>> +                    opp-hz = /bits/ 64 <10000000>;
>>> +                    required-opps = <&rpmhpd_opp_low_svs>;
>>> +                    opp-peak-kBps = <1000000 1>;
>>> +                };
>>> +
>>> +                /* GEN 1 x8 and GEN 2 X4 */
>> Inconsistent capitalization, please use lowercase 'x'
>>
>> [...]
>>
>>> +        pcie3_phy: phy@1be0000 {
>>> +            compatible = "qcom,x1e80100-qmp-gen4x8-pcie-phy";
>>> +            reg = <0 0x01be0000 0 0x10000>;
>>> +
>>> +            clocks = <&gcc GCC_PCIE_3_AUX_CLK>,
>> This clock doesn't belong here, the PHY is clocked by PHY_AUX
>>
>>> +                 <&gcc GCC_PCIE_3_CFG_AHB_CLK>,
>>> +                 <&rpmhcc RPMH_CXO_CLK>,
>> This is unnecessary as commented before
>>
>>> +                 <&gcc GCC_PCIE_3_PHY_RCHNG_CLK>,
>>> +                 <&gcc GCC_PCIE_3_PHY_AUX_CLK>,
>>> +                 <&gcc GCC_PCIE_3_PIPE_CLK>,
>>> +                 <&gcc GCC_PCIE_3_PIPEDIV2_CLK>,
>>> +                 <&tcsr TCSR_PCIE_8L_CLKREF_EN>;
>> This should be the 'ref' here
>>
>> Konrad

Re: [PATCH 4/8] arm64: dts: qcom: x1e80100: Add support for PCIe3 on x1e80100

[Krzysztof Kozlowski]

On Mon, Aug 26, 2024 at 11:36:27PM -0700, Qiang Yu wrote:
> Describe PCIe3 controller and PHY. Also add required system resources like
> regulators, clocks, interrupts and registers configuration for PCIe3.
> 
> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/x1e80100.dtsi | 205 ++++++++++++++++++++++++-

Why DTS is mixed with the drivers? This patchset is organized in
confusing way. Please use standard upstream submission process - DTS is
always the last in the patchset (or separate).


Best regards,
Krzysztof

[PATCH 5/8] dt-bindings: phy: qcom,sc8280xp-qmp-pcie-phy: Document the X1E80100 QMP PCIe PHY Gen4 x8

[Qiang Yu]

PCIe 3rd instance of X1E80100 support Gen 4x8 which needs different 8 lane
capable QMP PCIe PHY. Document Gen 4x8 PHY as separate module.

Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
---
 .../phy/qcom,sc8280xp-qmp-pcie-phy.yaml        | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

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 03dbd02cf9e7..e122657490b1 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-gen4x8-pcie-phy
 
   reg:
     minItems: 1
@@ -47,7 +48,7 @@ properties:
 
   clocks:
     minItems: 5
-    maxItems: 7
+    maxItems: 8
 
   clock-names:
     minItems: 5
@@ -59,6 +60,7 @@ properties:
       - const: pipe
       - const: pipediv2
       - const: phy_aux
+      - const: clkref_en
 
   power-domains:
     maxItems: 1
@@ -190,6 +192,19 @@ allOf:
         clock-names:
           minItems: 7
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,x1e80100-qmp-gen4x8-pcie-phy
+    then:
+      properties:
+        clocks:
+          minItems: 8
+        clock-names:
+          minItems: 8
+
   - if:
       properties:
         compatible:
@@ -198,6 +213,7 @@ allOf:
               - qcom,sm8550-qmp-gen4x2-pcie-phy
               - qcom,sm8650-qmp-gen4x2-pcie-phy
               - qcom,x1e80100-qmp-gen4x2-pcie-phy
+              - qcom,x1e80100-qmp-gen4x8-pcie-phy
     then:
       properties:
         resets:
-- 
2.34.1

Re: [PATCH 5/8] dt-bindings: phy: qcom,sc8280xp-qmp-pcie-phy: Document the X1E80100 QMP PCIe PHY Gen4 x8

[Krzysztof Kozlowski]

On Mon, Aug 26, 2024 at 11:36:28PM -0700, Qiang Yu wrote:
> PCIe 3rd instance of X1E80100 support Gen 4x8 which needs different 8 lane
> capable QMP PCIe PHY. Document Gen 4x8 PHY as separate module.
> 
> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> ---
>  .../phy/qcom,sc8280xp-qmp-pcie-phy.yaml        | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> 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 03dbd02cf9e7..e122657490b1 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-gen4x8-pcie-phy
>  
>    reg:
>      minItems: 1
> @@ -47,7 +48,7 @@ properties:
>  
>    clocks:
>      minItems: 5
> -    maxItems: 7
> +    maxItems: 8
>  
>    clock-names:
>      minItems: 5
> @@ -59,6 +60,7 @@ properties:
>        - const: pipe
>        - const: pipediv2
>        - const: phy_aux
> +      - const: clkref_en

That sounds like enabling clock ref, not the reference clock.

>  
>    power-domains:
>      maxItems: 1
> @@ -190,6 +192,19 @@ allOf:
>          clock-names:
>            minItems: 7

You need to now constrain other cases. Missing maxItems.

Best regards,
Krzysztof

[PATCH 6/8] clk: qcom: gcc-x1e80100: Fix halt_check for pipediv2 clocks

[Qiang Yu]

The pipediv2_clk's source from the same mux as pipe clock. So they have
same limitation, which is that the PHY sequence requires to enable these
local CBCs before the PHY is actually outputting a clock to them. This
means the clock won't actually turn on when we vote them. Hence, let's
skip the halt bit check of the pipediv2_clk, otherwise pipediv2_clk may
stuck at off state during bootup.

Suggested-by: Mike Tipton <quic_mdtipton@quicinc.com>
Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
---
 drivers/clk/qcom/gcc-x1e80100.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/qcom/gcc-x1e80100.c b/drivers/clk/qcom/gcc-x1e80100.c
index 80e90e31be33..ad35a8c37ca8 100644
--- a/drivers/clk/qcom/gcc-x1e80100.c
+++ b/drivers/clk/qcom/gcc-x1e80100.c
@@ -3123,7 +3123,7 @@ static struct clk_branch gcc_pcie_3_pipe_clk = {
 
 static struct clk_branch gcc_pcie_3_pipediv2_clk = {
 	.halt_reg = 0x58060,
-	.halt_check = BRANCH_HALT_VOTED,
+	.halt_check = BRANCH_HALT_SKIP,
 	.clkr = {
 		.enable_reg = 0x52020,
 		.enable_mask = BIT(5),
@@ -3248,7 +3248,7 @@ static struct clk_branch gcc_pcie_4_pipe_clk = {
 
 static struct clk_branch gcc_pcie_4_pipediv2_clk = {
 	.halt_reg = 0x6b054,
-	.halt_check = BRANCH_HALT_VOTED,
+	.halt_check = BRANCH_HALT_SKIP,
 	.clkr = {
 		.enable_reg = 0x52010,
 		.enable_mask = BIT(27),
@@ -3373,7 +3373,7 @@ static struct clk_branch gcc_pcie_5_pipe_clk = {
 
 static struct clk_branch gcc_pcie_5_pipediv2_clk = {
 	.halt_reg = 0x2f054,
-	.halt_check = BRANCH_HALT_VOTED,
+	.halt_check = BRANCH_HALT_SKIP,
 	.clkr = {
 		.enable_reg = 0x52018,
 		.enable_mask = BIT(19),
@@ -3511,7 +3511,7 @@ static struct clk_branch gcc_pcie_6a_pipe_clk = {
 
 static struct clk_branch gcc_pcie_6a_pipediv2_clk = {
 	.halt_reg = 0x31060,
-	.halt_check = BRANCH_HALT_VOTED,
+	.halt_check = BRANCH_HALT_SKIP,
 	.clkr = {
 		.enable_reg = 0x52018,
 		.enable_mask = BIT(28),
@@ -3649,7 +3649,7 @@ static struct clk_branch gcc_pcie_6b_pipe_clk = {
 
 static struct clk_branch gcc_pcie_6b_pipediv2_clk = {
 	.halt_reg = 0x8d060,
-	.halt_check = BRANCH_HALT_VOTED,
+	.halt_check = BRANCH_HALT_SKIP,
 	.clkr = {
 		.enable_reg = 0x52010,
 		.enable_mask = BIT(28),
-- 
2.34.1

Re: [PATCH 6/8] clk: qcom: gcc-x1e80100: Fix halt_check for pipediv2 clocks

[Konrad Dybcio]

On 27.08.2024 8:36 AM, Qiang Yu wrote:
> The pipediv2_clk's source from the same mux as pipe clock. So they have
> same limitation, which is that the PHY sequence requires to enable these
> local CBCs before the PHY is actually outputting a clock to them. This
> means the clock won't actually turn on when we vote them. Hence, let's
> skip the halt bit check of the pipediv2_clk, otherwise pipediv2_clk may
> stuck at off state during bootup.
> 
> Suggested-by: Mike Tipton <quic_mdtipton@quicinc.com>
> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> ---


Reviewed-by: Konrad Dybcio <konradybcio@kernel.org>

Konrad

[PATCH 7/8] arm64: dts: qcom: x1e80100-qcp: Add power supply and sideband signal for pcie3

[Qiang Yu]

Add perst, wake and clkreq gpio config. Add required power supply.

Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
---
 arch/arm64/boot/dts/qcom/x1e80100-qcp.dts | 116 ++++++++++++++++++++++
 1 file changed, 116 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts b/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
index 1c3a6a7b3ed6..0deb0c4bfea9 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
+++ b/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
@@ -254,6 +254,48 @@ vreg_nvme: regulator-nvme {
 		pinctrl-names = "default";
 		pinctrl-0 = <&nvme_reg_en>;
 	};
+
+	vreg_pcie_12v: regulator-pcie_12v {
+		compatible = "regulator-fixed";
+
+		regulator-name = "VREG_PCIE_12V";
+		regulator-min-microvolt = <12000000>;
+		regulator-max-microvolt = <12000000>;
+
+		gpio = <&pm8550ve_8_gpios 8 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&pcie_x8_12v>;
+	};
+
+	vreg_pcie_3v3_aux: regulator-pcie_3v3_aux {
+		compatible = "regulator-fixed";
+
+		regulator-name = "VREG_PCIE_3P3_AUX";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpio = <&pmc8380_3_gpios 8 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&pm_sde7_aux_3p3_en>;
+	};
+
+	vreg_pcie_3v3: regulator-pcie_3v3 {
+		compatible = "regulator-fixed";
+
+		regulator-name = "VREG_PCIE_3P3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpio = <&pmc8380_3_gpios 6 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&pm_sde7_main_3p3_en>;
+	};
 };
 
 &apps_rsc {
@@ -667,6 +709,57 @@ &mdss_dp3_phy {
 	status = "okay";
 };
 
+&pm8550ve_8_gpios {
+	pcie_x8_12v: pcie_x8_12v_on {
+		pins = "gpio8";
+		function = "normal";
+		output-enable;
+		output-high;
+		bias-pull-down;
+		power-source = <0>;
+	};
+};
+
+&pmc8380_3_gpios {
+	pm_sde7_aux_3p3_en: pm_sde7_aux_3p3 {
+		pins = "gpio8";
+		function = "normal";
+		output-enable;
+		output-high;
+		bias-pull-down;
+		power-source = <0>;
+	};
+
+	pm_sde7_main_3p3_en: pm_sde7_main_3p3 {
+		pins = "gpio6";
+		function = "normal";
+		output-enable;
+		output-high;
+		bias-pull-down;
+		power-source = <0>;
+	};
+};
+
+&pcie3 {
+	vpcie12v-supply = <&vreg_pcie_12v>;
+	vpcie3v3-supply = <&vreg_pcie_3v3>;
+	vpcie3v3aux-supply = <&vreg_pcie_3v3_aux>;
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&pcie3_default>;
+	perst-gpios = <&tlmm 143 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 145 GPIO_ACTIVE_LOW>;
+	status = "okay";
+};
+
+&pcie3_phy {
+	vdda-phy-supply = <&vreg_l3j_0p8>;
+	vdda-pll-supply = <&vreg_l3e_1p2>;
+	vdda-qref-supply = <&vreg_l3c_0p8>;
+
+	status = "okay";
+};
+
 &pcie4 {
 	perst-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
 	wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
@@ -824,6 +917,29 @@ nvme_reg_en: nvme-reg-en-state {
 		bias-disable;
 	};
 
+	pcie3_default: pcie3-default-state {
+		clkreq-n-pins {
+			pins = "gpio144";
+			function = "pcie3_clk";
+			drive-strength = <2>;
+			bias-pull-up;
+		};
+
+		perst-n-pins {
+			pins = "gpio143";
+			function = "gpio";
+			drive-strength = <2>;
+			bias-pull-down;
+		};
+
+		wake-n-pins {
+		       pins = "gpio145";
+		       function = "gpio";
+		       drive-strength = <2>;
+		       bias-pull-up;
+		};
+	};
+
 	pcie4_default: pcie4-default-state {
 		clkreq-n-pins {
 			pins = "gpio147";
-- 
2.34.1

Re: [PATCH 7/8] arm64: dts: qcom: x1e80100-qcp: Add power supply and sideband signal for pcie3

[Krzysztof Kozlowski]

On Mon, Aug 26, 2024 at 11:36:30PM -0700, Qiang Yu wrote:
> Add perst, wake and clkreq gpio config. Add required power supply.
> 
> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/x1e80100-qcp.dts | 116 ++++++++++++++++++++++
>  1 file changed, 116 insertions(+)
> 

Really, driver cannot depend on this patch. That's a no go.

> diff --git a/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts b/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
> index 1c3a6a7b3ed6..0deb0c4bfea9 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
> +++ b/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
> @@ -254,6 +254,48 @@ vreg_nvme: regulator-nvme {
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&nvme_reg_en>;
>  	};
> +
> +	vreg_pcie_12v: regulator-pcie_12v {
> +		compatible = "regulator-fixed";
> +
> +		regulator-name = "VREG_PCIE_12V";
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +
> +		gpio = <&pm8550ve_8_gpios 8 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pcie_x8_12v>;
> +	};
> +
> +	vreg_pcie_3v3_aux: regulator-pcie_3v3_aux {

Please follow DTS coding style.

> +		compatible = "regulator-fixed";
> +
> +		regulator-name = "VREG_PCIE_3P3_AUX";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +
> +		gpio = <&pmc8380_3_gpios 8 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pm_sde7_aux_3p3_en>;
> +	};
> +
> +	vreg_pcie_3v3: regulator-pcie_3v3 {
> +		compatible = "regulator-fixed";
> +
> +		regulator-name = "VREG_PCIE_3P3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +
> +		gpio = <&pmc8380_3_gpios 6 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pm_sde7_main_3p3_en>;
> +	};
>  };
>  
>  &apps_rsc {
> @@ -667,6 +709,57 @@ &mdss_dp3_phy {
>  	status = "okay";
>  };
>  
> +&pm8550ve_8_gpios {
> +	pcie_x8_12v: pcie_x8_12v_on {

Never tested.

Best regards,
Krzysztof

[PATCH 8/8] PCI: qcom: Add support to PCIe slot power supplies

[Qiang Yu]

On platform x1e80100 QCP, PCIe3 is a standard x8 form factor. Hence, add
support to use 3.3v, 3.3v aux and 12v regulators.

Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 52 +++++++++++++++++++++++++-
 1 file changed, 50 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 6f953e32d990..59fb415dfeeb 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -248,6 +248,8 @@ struct qcom_pcie_cfg {
 	bool no_l0s;
 };
 
+#define QCOM_PCIE_SLOT_MAX_SUPPLIES			3
+
 struct qcom_pcie {
 	struct dw_pcie *pci;
 	void __iomem *parf;			/* DT parf */
@@ -260,6 +262,7 @@ struct qcom_pcie {
 	struct icc_path *icc_cpu;
 	const struct qcom_pcie_cfg *cfg;
 	struct dentry *debugfs;
+	struct regulator_bulk_data slot_supplies[QCOM_PCIE_SLOT_MAX_SUPPLIES];
 	bool suspended;
 	bool use_pm_opp;
 };
@@ -1174,6 +1177,41 @@ static int qcom_pcie_link_up(struct dw_pcie *pci)
 	return !!(val & PCI_EXP_LNKSTA_DLLLA);
 }
 
+static int qcom_pcie_enable_slot_supplies(struct qcom_pcie *pcie)
+{
+	struct dw_pcie *pci = pcie->pci;
+	int ret;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(pcie->slot_supplies),
+				    pcie->slot_supplies);
+	if (ret < 0)
+		dev_err(pci->dev, "Failed to enable slot regulators\n");
+
+	return ret;
+}
+
+static void qcom_pcie_disable_slot_supplies(struct qcom_pcie *pcie)
+{
+	regulator_bulk_disable(ARRAY_SIZE(pcie->slot_supplies),
+			       pcie->slot_supplies);
+}
+
+static int qcom_pcie_get_slot_supplies(struct qcom_pcie *pcie)
+{
+	struct dw_pcie *pci = pcie->pci;
+	int ret;
+
+	pcie->slot_supplies[0].supply = "vpcie12v";
+	pcie->slot_supplies[1].supply = "vpcie3v3";
+	pcie->slot_supplies[2].supply = "vpcie3v3aux";
+	ret = devm_regulator_bulk_get(pci->dev, ARRAY_SIZE(pcie->slot_supplies),
+				      pcie->slot_supplies);
+	if (ret < 0)
+		dev_err(pci->dev, "Failed to get slot regulators\n");
+
+	return ret;
+}
+
 static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -1182,10 +1220,14 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
 
 	qcom_ep_reset_assert(pcie);
 
-	ret = pcie->cfg->ops->init(pcie);
+	ret = qcom_pcie_enable_slot_supplies(pcie);
 	if (ret)
 		return ret;
 
+	ret = pcie->cfg->ops->init(pcie);
+	if (ret)
+		goto err_disable_slot;
+
 	ret = phy_set_mode_ext(pcie->phy, PHY_MODE_PCIE, PHY_MODE_PCIE_RC);
 	if (ret)
 		goto err_deinit;
@@ -1216,7 +1258,8 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
 	phy_power_off(pcie->phy);
 err_deinit:
 	pcie->cfg->ops->deinit(pcie);
-
+err_disable_slot:
+	qcom_pcie_disable_slot_supplies(pcie);
 	return ret;
 }
 
@@ -1228,6 +1271,7 @@ static void qcom_pcie_host_deinit(struct dw_pcie_rp *pp)
 	qcom_ep_reset_assert(pcie);
 	phy_power_off(pcie->phy);
 	pcie->cfg->ops->deinit(pcie);
+	qcom_pcie_disable_slot_supplies(pcie);
 }
 
 static void qcom_pcie_host_post_init(struct dw_pcie_rp *pp)
@@ -1602,6 +1646,10 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 			goto err_pm_runtime_put;
 	}
 
+	ret = qcom_pcie_get_slot_supplies(pcie);
+	if (ret)
+		goto err_pm_runtime_put;
+
 	ret = pcie->cfg->ops->get_resources(pcie);
 	if (ret)
 		goto err_pm_runtime_put;
-- 
2.34.1

Re: [PATCH 8/8] PCI: qcom: Add support to PCIe slot power supplies

[Konrad Dybcio]

On 27.08.2024 8:36 AM, Qiang Yu wrote:
> On platform x1e80100 QCP, PCIe3 is a standard x8 form factor. Hence, add
> support to use 3.3v, 3.3v aux and 12v regulators.
> 
> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 52 +++++++++++++++++++++++++-
>  1 file changed, 50 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 6f953e32d990..59fb415dfeeb 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -248,6 +248,8 @@ struct qcom_pcie_cfg {
>  	bool no_l0s;
>  };
>  
> +#define QCOM_PCIE_SLOT_MAX_SUPPLIES			3
> +
>  struct qcom_pcie {
>  	struct dw_pcie *pci;
>  	void __iomem *parf;			/* DT parf */
> @@ -260,6 +262,7 @@ struct qcom_pcie {
>  	struct icc_path *icc_cpu;
>  	const struct qcom_pcie_cfg *cfg;
>  	struct dentry *debugfs;
> +	struct regulator_bulk_data slot_supplies[QCOM_PCIE_SLOT_MAX_SUPPLIES];
>  	bool suspended;
>  	bool use_pm_opp;
>  };
> @@ -1174,6 +1177,41 @@ static int qcom_pcie_link_up(struct dw_pcie *pci)
>  	return !!(val & PCI_EXP_LNKSTA_DLLLA);
>  }
>  
> +static int qcom_pcie_enable_slot_supplies(struct qcom_pcie *pcie)
> +{
> +	struct dw_pcie *pci = pcie->pci;
> +	int ret;
> +
> +	ret = regulator_bulk_enable(ARRAY_SIZE(pcie->slot_supplies),
> +				    pcie->slot_supplies);
> +	if (ret < 0)
> +		dev_err(pci->dev, "Failed to enable slot regulators\n");

return dev_err_probe would be a good call.. probably more so below,
but won't hurt to use here too

> +
> +	return ret;
> +}
> +
> +static void qcom_pcie_disable_slot_supplies(struct qcom_pcie *pcie)
> +{
> +	regulator_bulk_disable(ARRAY_SIZE(pcie->slot_supplies),
> +			       pcie->slot_supplies);
> +}

This I feel like is overly abstracted

Konrad

Re: [PATCH 8/8] PCI: qcom: Add support to PCIe slot power supplies

[Dmitry Baryshkov]

On Tue, 27 Aug 2024 at 09:36, Qiang Yu <quic_qianyu@quicinc.com> wrote:
>
> On platform x1e80100 QCP, PCIe3 is a standard x8 form factor. Hence, add
> support to use 3.3v, 3.3v aux and 12v regulators.

First of all, I don't see corresponding bindings change.

Second, these supplies power up the slot, not the host controller
itself. As such these supplies do not belong to the host controller
entry. Please consider using the pwrseq framework instead.

>
> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 52 +++++++++++++++++++++++++-
>  1 file changed, 50 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 6f953e32d990..59fb415dfeeb 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -248,6 +248,8 @@ struct qcom_pcie_cfg {
>         bool no_l0s;
>  };
>
> +#define QCOM_PCIE_SLOT_MAX_SUPPLIES                    3
> +
>  struct qcom_pcie {
>         struct dw_pcie *pci;
>         void __iomem *parf;                     /* DT parf */
> @@ -260,6 +262,7 @@ struct qcom_pcie {
>         struct icc_path *icc_cpu;
>         const struct qcom_pcie_cfg *cfg;
>         struct dentry *debugfs;
> +       struct regulator_bulk_data slot_supplies[QCOM_PCIE_SLOT_MAX_SUPPLIES];
>         bool suspended;
>         bool use_pm_opp;
>  };
> @@ -1174,6 +1177,41 @@ static int qcom_pcie_link_up(struct dw_pcie *pci)
>         return !!(val & PCI_EXP_LNKSTA_DLLLA);
>  }
>
> +static int qcom_pcie_enable_slot_supplies(struct qcom_pcie *pcie)
> +{
> +       struct dw_pcie *pci = pcie->pci;
> +       int ret;
> +
> +       ret = regulator_bulk_enable(ARRAY_SIZE(pcie->slot_supplies),
> +                                   pcie->slot_supplies);
> +       if (ret < 0)
> +               dev_err(pci->dev, "Failed to enable slot regulators\n");
> +
> +       return ret;
> +}
> +
> +static void qcom_pcie_disable_slot_supplies(struct qcom_pcie *pcie)
> +{
> +       regulator_bulk_disable(ARRAY_SIZE(pcie->slot_supplies),
> +                              pcie->slot_supplies);
> +}
> +
> +static int qcom_pcie_get_slot_supplies(struct qcom_pcie *pcie)
> +{
> +       struct dw_pcie *pci = pcie->pci;
> +       int ret;
> +
> +       pcie->slot_supplies[0].supply = "vpcie12v";
> +       pcie->slot_supplies[1].supply = "vpcie3v3";
> +       pcie->slot_supplies[2].supply = "vpcie3v3aux";
> +       ret = devm_regulator_bulk_get(pci->dev, ARRAY_SIZE(pcie->slot_supplies),
> +                                     pcie->slot_supplies);
> +       if (ret < 0)
> +               dev_err(pci->dev, "Failed to get slot regulators\n");
> +
> +       return ret;
> +}
> +
>  static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
>  {
>         struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -1182,10 +1220,14 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
>
>         qcom_ep_reset_assert(pcie);
>
> -       ret = pcie->cfg->ops->init(pcie);
> +       ret = qcom_pcie_enable_slot_supplies(pcie);
>         if (ret)
>                 return ret;
>
> +       ret = pcie->cfg->ops->init(pcie);
> +       if (ret)
> +               goto err_disable_slot;
> +
>         ret = phy_set_mode_ext(pcie->phy, PHY_MODE_PCIE, PHY_MODE_PCIE_RC);
>         if (ret)
>                 goto err_deinit;
> @@ -1216,7 +1258,8 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
>         phy_power_off(pcie->phy);
>  err_deinit:
>         pcie->cfg->ops->deinit(pcie);
> -
> +err_disable_slot:
> +       qcom_pcie_disable_slot_supplies(pcie);
>         return ret;
>  }
>
> @@ -1228,6 +1271,7 @@ static void qcom_pcie_host_deinit(struct dw_pcie_rp *pp)
>         qcom_ep_reset_assert(pcie);
>         phy_power_off(pcie->phy);
>         pcie->cfg->ops->deinit(pcie);
> +       qcom_pcie_disable_slot_supplies(pcie);
>  }
>
>  static void qcom_pcie_host_post_init(struct dw_pcie_rp *pp)
> @@ -1602,6 +1646,10 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>                         goto err_pm_runtime_put;
>         }
>
> +       ret = qcom_pcie_get_slot_supplies(pcie);
> +       if (ret)
> +               goto err_pm_runtime_put;
> +
>         ret = pcie->cfg->ops->get_resources(pcie);
>         if (ret)
>                 goto err_pm_runtime_put;
> --
> 2.34.1
>


-- 
With best wishes
Dmitry

Re: [PATCH 8/8] PCI: qcom: Add support to PCIe slot power supplies

[Manivannan Sadhasivam]

On Tue, Aug 27, 2024 at 02:44:09PM +0300, Dmitry Baryshkov wrote:
> On Tue, 27 Aug 2024 at 09:36, Qiang Yu <quic_qianyu@quicinc.com> wrote:
> >
> > On platform x1e80100 QCP, PCIe3 is a standard x8 form factor. Hence, add
> > support to use 3.3v, 3.3v aux and 12v regulators.
> 
> First of all, I don't see corresponding bindings change.
> 
> Second, these supplies power up the slot, not the host controller
> itself. As such these supplies do not belong to the host controller
> entry. Please consider using the pwrseq framework instead.
> 

Indeed. For legacy reasons, slot power supplies were populated in the host
bridge node itself until recently Rob started objecting it [1]. And it makes
real sense to put these supplies in the root port node and handle them in the
relevant driver.

I'm still evaluating whether the handling should be done in the portdrv or
pwrctl driver, but haven't reached the conclusion. Pwrctl seems to be the ideal
choice, but I see a few issues related to handling the OF node for the root
port.

Hope I'll come to a conclusion in the next few days and will update this thread.

- Mani

[1] https://lore.kernel.org/lkml/20240604235806.GA1903493-robh@kernel.org/

> >
> > Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> > ---
> >  drivers/pci/controller/dwc/pcie-qcom.c | 52 +++++++++++++++++++++++++-
> >  1 file changed, 50 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > index 6f953e32d990..59fb415dfeeb 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -248,6 +248,8 @@ struct qcom_pcie_cfg {
> >         bool no_l0s;
> >  };
> >
> > +#define QCOM_PCIE_SLOT_MAX_SUPPLIES                    3
> > +
> >  struct qcom_pcie {
> >         struct dw_pcie *pci;
> >         void __iomem *parf;                     /* DT parf */
> > @@ -260,6 +262,7 @@ struct qcom_pcie {
> >         struct icc_path *icc_cpu;
> >         const struct qcom_pcie_cfg *cfg;
> >         struct dentry *debugfs;
> > +       struct regulator_bulk_data slot_supplies[QCOM_PCIE_SLOT_MAX_SUPPLIES];
> >         bool suspended;
> >         bool use_pm_opp;
> >  };
> > @@ -1174,6 +1177,41 @@ static int qcom_pcie_link_up(struct dw_pcie *pci)
> >         return !!(val & PCI_EXP_LNKSTA_DLLLA);
> >  }
> >
> > +static int qcom_pcie_enable_slot_supplies(struct qcom_pcie *pcie)
> > +{
> > +       struct dw_pcie *pci = pcie->pci;
> > +       int ret;
> > +
> > +       ret = regulator_bulk_enable(ARRAY_SIZE(pcie->slot_supplies),
> > +                                   pcie->slot_supplies);
> > +       if (ret < 0)
> > +               dev_err(pci->dev, "Failed to enable slot regulators\n");
> > +
> > +       return ret;
> > +}
> > +
> > +static void qcom_pcie_disable_slot_supplies(struct qcom_pcie *pcie)
> > +{
> > +       regulator_bulk_disable(ARRAY_SIZE(pcie->slot_supplies),
> > +                              pcie->slot_supplies);
> > +}
> > +
> > +static int qcom_pcie_get_slot_supplies(struct qcom_pcie *pcie)
> > +{
> > +       struct dw_pcie *pci = pcie->pci;
> > +       int ret;
> > +
> > +       pcie->slot_supplies[0].supply = "vpcie12v";
> > +       pcie->slot_supplies[1].supply = "vpcie3v3";
> > +       pcie->slot_supplies[2].supply = "vpcie3v3aux";
> > +       ret = devm_regulator_bulk_get(pci->dev, ARRAY_SIZE(pcie->slot_supplies),
> > +                                     pcie->slot_supplies);
> > +       if (ret < 0)
> > +               dev_err(pci->dev, "Failed to get slot regulators\n");
> > +
> > +       return ret;
> > +}
> > +
> >  static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
> >  {
> >         struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > @@ -1182,10 +1220,14 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
> >
> >         qcom_ep_reset_assert(pcie);
> >
> > -       ret = pcie->cfg->ops->init(pcie);
> > +       ret = qcom_pcie_enable_slot_supplies(pcie);
> >         if (ret)
> >                 return ret;
> >
> > +       ret = pcie->cfg->ops->init(pcie);
> > +       if (ret)
> > +               goto err_disable_slot;
> > +
> >         ret = phy_set_mode_ext(pcie->phy, PHY_MODE_PCIE, PHY_MODE_PCIE_RC);
> >         if (ret)
> >                 goto err_deinit;
> > @@ -1216,7 +1258,8 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
> >         phy_power_off(pcie->phy);
> >  err_deinit:
> >         pcie->cfg->ops->deinit(pcie);
> > -
> > +err_disable_slot:
> > +       qcom_pcie_disable_slot_supplies(pcie);
> >         return ret;
> >  }
> >
> > @@ -1228,6 +1271,7 @@ static void qcom_pcie_host_deinit(struct dw_pcie_rp *pp)
> >         qcom_ep_reset_assert(pcie);
> >         phy_power_off(pcie->phy);
> >         pcie->cfg->ops->deinit(pcie);
> > +       qcom_pcie_disable_slot_supplies(pcie);
> >  }
> >
> >  static void qcom_pcie_host_post_init(struct dw_pcie_rp *pp)
> > @@ -1602,6 +1646,10 @@ static int qcom_pcie_probe(struct platform_device *pdev)
> >                         goto err_pm_runtime_put;
> >         }
> >
> > +       ret = qcom_pcie_get_slot_supplies(pcie);
> > +       if (ret)
> > +               goto err_pm_runtime_put;
> > +
> >         ret = pcie->cfg->ops->get_resources(pcie);
> >         if (ret)
> >                 goto err_pm_runtime_put;
> > --
> > 2.34.1
> >
> 
> 
> -- 
> With best wishes
> Dmitry

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 8/8] PCI: qcom: Add support to PCIe slot power supplies

[Qiang Yu]

On 8/28/2024 12:58 AM, Manivannan Sadhasivam wrote:
> On Tue, Aug 27, 2024 at 02:44:09PM +0300, Dmitry Baryshkov wrote:
>> On Tue, 27 Aug 2024 at 09:36, Qiang Yu <quic_qianyu@quicinc.com> wrote:
>>> On platform x1e80100 QCP, PCIe3 is a standard x8 form factor. Hence, add
>>> support to use 3.3v, 3.3v aux and 12v regulators.
>> First of all, I don't see corresponding bindings change.
>>
>> Second, these supplies power up the slot, not the host controller
>> itself. As such these supplies do not belong to the host controller
>> entry. Please consider using the pwrseq framework instead.
>>
> Indeed. For legacy reasons, slot power supplies were populated in the host
> bridge node itself until recently Rob started objecting it [1]. And it makes
> real sense to put these supplies in the root port node and handle them in the
> relevant driver.
>
> I'm still evaluating whether the handling should be done in the portdrv or
> pwrctl driver, but haven't reached the conclusion. Pwrctl seems to be the ideal
> choice, but I see a few issues related to handling the OF node for the root
> port.
>
> Hope I'll come to a conclusion in the next few days and will update this thread.
>
> - Mani
>
> [1] https://lore.kernel.org/lkml/20240604235806.GA1903493-robh@kernel.org/
Hi Mani, do you have any updates?

Thanks,
Qiang
>
>>> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
>>> ---
>>>   drivers/pci/controller/dwc/pcie-qcom.c | 52 +++++++++++++++++++++++++-
>>>   1 file changed, 50 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>>> index 6f953e32d990..59fb415dfeeb 100644
>>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>>> @@ -248,6 +248,8 @@ struct qcom_pcie_cfg {
>>>          bool no_l0s;
>>>   };
>>>
>>> +#define QCOM_PCIE_SLOT_MAX_SUPPLIES                    3
>>> +
>>>   struct qcom_pcie {
>>>          struct dw_pcie *pci;
>>>          void __iomem *parf;                     /* DT parf */
>>> @@ -260,6 +262,7 @@ struct qcom_pcie {
>>>          struct icc_path *icc_cpu;
>>>          const struct qcom_pcie_cfg *cfg;
>>>          struct dentry *debugfs;
>>> +       struct regulator_bulk_data slot_supplies[QCOM_PCIE_SLOT_MAX_SUPPLIES];
>>>          bool suspended;
>>>          bool use_pm_opp;
>>>   };
>>> @@ -1174,6 +1177,41 @@ static int qcom_pcie_link_up(struct dw_pcie *pci)
>>>          return !!(val & PCI_EXP_LNKSTA_DLLLA);
>>>   }
>>>
>>> +static int qcom_pcie_enable_slot_supplies(struct qcom_pcie *pcie)
>>> +{
>>> +       struct dw_pcie *pci = pcie->pci;
>>> +       int ret;
>>> +
>>> +       ret = regulator_bulk_enable(ARRAY_SIZE(pcie->slot_supplies),
>>> +                                   pcie->slot_supplies);
>>> +       if (ret < 0)
>>> +               dev_err(pci->dev, "Failed to enable slot regulators\n");
>>> +
>>> +       return ret;
>>> +}
>>> +
>>> +static void qcom_pcie_disable_slot_supplies(struct qcom_pcie *pcie)
>>> +{
>>> +       regulator_bulk_disable(ARRAY_SIZE(pcie->slot_supplies),
>>> +                              pcie->slot_supplies);
>>> +}
>>> +
>>> +static int qcom_pcie_get_slot_supplies(struct qcom_pcie *pcie)
>>> +{
>>> +       struct dw_pcie *pci = pcie->pci;
>>> +       int ret;
>>> +
>>> +       pcie->slot_supplies[0].supply = "vpcie12v";
>>> +       pcie->slot_supplies[1].supply = "vpcie3v3";
>>> +       pcie->slot_supplies[2].supply = "vpcie3v3aux";
>>> +       ret = devm_regulator_bulk_get(pci->dev, ARRAY_SIZE(pcie->slot_supplies),
>>> +                                     pcie->slot_supplies);
>>> +       if (ret < 0)
>>> +               dev_err(pci->dev, "Failed to get slot regulators\n");
>>> +
>>> +       return ret;
>>> +}
>>> +
>>>   static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
>>>   {
>>>          struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>>> @@ -1182,10 +1220,14 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
>>>
>>>          qcom_ep_reset_assert(pcie);
>>>
>>> -       ret = pcie->cfg->ops->init(pcie);
>>> +       ret = qcom_pcie_enable_slot_supplies(pcie);
>>>          if (ret)
>>>                  return ret;
>>>
>>> +       ret = pcie->cfg->ops->init(pcie);
>>> +       if (ret)
>>> +               goto err_disable_slot;
>>> +
>>>          ret = phy_set_mode_ext(pcie->phy, PHY_MODE_PCIE, PHY_MODE_PCIE_RC);
>>>          if (ret)
>>>                  goto err_deinit;
>>> @@ -1216,7 +1258,8 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
>>>          phy_power_off(pcie->phy);
>>>   err_deinit:
>>>          pcie->cfg->ops->deinit(pcie);
>>> -
>>> +err_disable_slot:
>>> +       qcom_pcie_disable_slot_supplies(pcie);
>>>          return ret;
>>>   }
>>>
>>> @@ -1228,6 +1271,7 @@ static void qcom_pcie_host_deinit(struct dw_pcie_rp *pp)
>>>          qcom_ep_reset_assert(pcie);
>>>          phy_power_off(pcie->phy);
>>>          pcie->cfg->ops->deinit(pcie);
>>> +       qcom_pcie_disable_slot_supplies(pcie);
>>>   }
>>>
>>>   static void qcom_pcie_host_post_init(struct dw_pcie_rp *pp)
>>> @@ -1602,6 +1646,10 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>>>                          goto err_pm_runtime_put;
>>>          }
>>>
>>> +       ret = qcom_pcie_get_slot_supplies(pcie);
>>> +       if (ret)
>>> +               goto err_pm_runtime_put;
>>> +
>>>          ret = pcie->cfg->ops->get_resources(pcie);
>>>          if (ret)
>>>                  goto err_pm_runtime_put;
>>> --
>>> 2.34.1
>>>
>>
>> -- 
>> With best wishes
>> Dmitry

Re: [PATCH 8/8] PCI: qcom: Add support to PCIe slot power supplies

[Manivannan Sadhasivam]

On Wed, Sep 11, 2024 at 04:17:41PM +0800, Qiang Yu wrote:
> 
> On 8/28/2024 12:58 AM, Manivannan Sadhasivam wrote:
> > On Tue, Aug 27, 2024 at 02:44:09PM +0300, Dmitry Baryshkov wrote:
> > > On Tue, 27 Aug 2024 at 09:36, Qiang Yu <quic_qianyu@quicinc.com> wrote:
> > > > On platform x1e80100 QCP, PCIe3 is a standard x8 form factor. Hence, add
> > > > support to use 3.3v, 3.3v aux and 12v regulators.
> > > First of all, I don't see corresponding bindings change.
> > > 
> > > Second, these supplies power up the slot, not the host controller
> > > itself. As such these supplies do not belong to the host controller
> > > entry. Please consider using the pwrseq framework instead.
> > > 
> > Indeed. For legacy reasons, slot power supplies were populated in the host
> > bridge node itself until recently Rob started objecting it [1]. And it makes
> > real sense to put these supplies in the root port node and handle them in the
> > relevant driver.
> > 
> > I'm still evaluating whether the handling should be done in the portdrv or
> > pwrctl driver, but haven't reached the conclusion. Pwrctl seems to be the ideal
> > choice, but I see a few issues related to handling the OF node for the root
> > port.
> > 
> > Hope I'll come to a conclusion in the next few days and will update this thread.
> > 
> > - Mani
> > 
> > [1] https://lore.kernel.org/lkml/20240604235806.GA1903493-robh@kernel.org/
> Hi Mani, do you have any updates?
> 

I'm working with Bartosz to add a new pwrctl driver for rootports. And we are
debugging an issue currently. Unfortunately, the progress is very slow as I'm on
vacation still.

Will post the patches once it got resolved.

- Mani

> Thanks,
> Qiang
> > 
> > > > Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
> > > > ---
> > > >   drivers/pci/controller/dwc/pcie-qcom.c | 52 +++++++++++++++++++++++++-
> > > >   1 file changed, 50 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > index 6f953e32d990..59fb415dfeeb 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > @@ -248,6 +248,8 @@ struct qcom_pcie_cfg {
> > > >          bool no_l0s;
> > > >   };
> > > > 
> > > > +#define QCOM_PCIE_SLOT_MAX_SUPPLIES                    3
> > > > +
> > > >   struct qcom_pcie {
> > > >          struct dw_pcie *pci;
> > > >          void __iomem *parf;                     /* DT parf */
> > > > @@ -260,6 +262,7 @@ struct qcom_pcie {
> > > >          struct icc_path *icc_cpu;
> > > >          const struct qcom_pcie_cfg *cfg;
> > > >          struct dentry *debugfs;
> > > > +       struct regulator_bulk_data slot_supplies[QCOM_PCIE_SLOT_MAX_SUPPLIES];
> > > >          bool suspended;
> > > >          bool use_pm_opp;
> > > >   };
> > > > @@ -1174,6 +1177,41 @@ static int qcom_pcie_link_up(struct dw_pcie *pci)
> > > >          return !!(val & PCI_EXP_LNKSTA_DLLLA);
> > > >   }
> > > > 
> > > > +static int qcom_pcie_enable_slot_supplies(struct qcom_pcie *pcie)
> > > > +{
> > > > +       struct dw_pcie *pci = pcie->pci;
> > > > +       int ret;
> > > > +
> > > > +       ret = regulator_bulk_enable(ARRAY_SIZE(pcie->slot_supplies),
> > > > +                                   pcie->slot_supplies);
> > > > +       if (ret < 0)
> > > > +               dev_err(pci->dev, "Failed to enable slot regulators\n");
> > > > +
> > > > +       return ret;
> > > > +}
> > > > +
> > > > +static void qcom_pcie_disable_slot_supplies(struct qcom_pcie *pcie)
> > > > +{
> > > > +       regulator_bulk_disable(ARRAY_SIZE(pcie->slot_supplies),
> > > > +                              pcie->slot_supplies);
> > > > +}
> > > > +
> > > > +static int qcom_pcie_get_slot_supplies(struct qcom_pcie *pcie)
> > > > +{
> > > > +       struct dw_pcie *pci = pcie->pci;
> > > > +       int ret;
> > > > +
> > > > +       pcie->slot_supplies[0].supply = "vpcie12v";
> > > > +       pcie->slot_supplies[1].supply = "vpcie3v3";
> > > > +       pcie->slot_supplies[2].supply = "vpcie3v3aux";
> > > > +       ret = devm_regulator_bulk_get(pci->dev, ARRAY_SIZE(pcie->slot_supplies),
> > > > +                                     pcie->slot_supplies);
> > > > +       if (ret < 0)
> > > > +               dev_err(pci->dev, "Failed to get slot regulators\n");
> > > > +
> > > > +       return ret;
> > > > +}
> > > > +
> > > >   static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
> > > >   {
> > > >          struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > > > @@ -1182,10 +1220,14 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
> > > > 
> > > >          qcom_ep_reset_assert(pcie);
> > > > 
> > > > -       ret = pcie->cfg->ops->init(pcie);
> > > > +       ret = qcom_pcie_enable_slot_supplies(pcie);
> > > >          if (ret)
> > > >                  return ret;
> > > > 
> > > > +       ret = pcie->cfg->ops->init(pcie);
> > > > +       if (ret)
> > > > +               goto err_disable_slot;
> > > > +
> > > >          ret = phy_set_mode_ext(pcie->phy, PHY_MODE_PCIE, PHY_MODE_PCIE_RC);
> > > >          if (ret)
> > > >                  goto err_deinit;
> > > > @@ -1216,7 +1258,8 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
> > > >          phy_power_off(pcie->phy);
> > > >   err_deinit:
> > > >          pcie->cfg->ops->deinit(pcie);
> > > > -
> > > > +err_disable_slot:
> > > > +       qcom_pcie_disable_slot_supplies(pcie);
> > > >          return ret;
> > > >   }
> > > > 
> > > > @@ -1228,6 +1271,7 @@ static void qcom_pcie_host_deinit(struct dw_pcie_rp *pp)
> > > >          qcom_ep_reset_assert(pcie);
> > > >          phy_power_off(pcie->phy);
> > > >          pcie->cfg->ops->deinit(pcie);
> > > > +       qcom_pcie_disable_slot_supplies(pcie);
> > > >   }
> > > > 
> > > >   static void qcom_pcie_host_post_init(struct dw_pcie_rp *pp)
> > > > @@ -1602,6 +1646,10 @@ static int qcom_pcie_probe(struct platform_device *pdev)
> > > >                          goto err_pm_runtime_put;
> > > >          }
> > > > 
> > > > +       ret = qcom_pcie_get_slot_supplies(pcie);
> > > > +       if (ret)
> > > > +               goto err_pm_runtime_put;
> > > > +
> > > >          ret = pcie->cfg->ops->get_resources(pcie);
> > > >          if (ret)
> > > >                  goto err_pm_runtime_put;
> > > > --
> > > > 2.34.1
> > > > 
> > > 
> > > -- 
> > > With best wishes
> > > Dmitry

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 8/8] PCI: qcom: Add support to PCIe slot power supplies

[Qiang Yu]

On 9/11/2024 11:32 PM, Manivannan Sadhasivam wrote:
> On Wed, Sep 11, 2024 at 04:17:41PM +0800, Qiang Yu wrote:
>> On 8/28/2024 12:58 AM, Manivannan Sadhasivam wrote:
>>> On Tue, Aug 27, 2024 at 02:44:09PM +0300, Dmitry Baryshkov wrote:
>>>> On Tue, 27 Aug 2024 at 09:36, Qiang Yu <quic_qianyu@quicinc.com> wrote:
>>>>> On platform x1e80100 QCP, PCIe3 is a standard x8 form factor. Hence, add
>>>>> support to use 3.3v, 3.3v aux and 12v regulators.
>>>> First of all, I don't see corresponding bindings change.
>>>>
>>>> Second, these supplies power up the slot, not the host controller
>>>> itself. As such these supplies do not belong to the host controller
>>>> entry. Please consider using the pwrseq framework instead.
>>>>
>>> Indeed. For legacy reasons, slot power supplies were populated in the host
>>> bridge node itself until recently Rob started objecting it [1]. And it makes
>>> real sense to put these supplies in the root port node and handle them in the
>>> relevant driver.
>>>
>>> I'm still evaluating whether the handling should be done in the portdrv or
>>> pwrctl driver, but haven't reached the conclusion. Pwrctl seems to be the ideal
>>> choice, but I see a few issues related to handling the OF node for the root
>>> port.
>>>
>>> Hope I'll come to a conclusion in the next few days and will update this thread.
>>>
>>> - Mani
>>>
>>> [1] https://lore.kernel.org/lkml/20240604235806.GA1903493-robh@kernel.org/
>> Hi Mani, do you have any updates?
>>
> I'm working with Bartosz to add a new pwrctl driver for rootports. And we are
> debugging an issue currently. Unfortunately, the progress is very slow as I'm on
> vacation still.
>
> Will post the patches once it got resolved.
>
> - Mani
OK, thanks for your update.

Thanks,
Qiang
>> Thanks,
>> Qiang
>>>>> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
>>>>> ---
>>>>>    drivers/pci/controller/dwc/pcie-qcom.c | 52 +++++++++++++++++++++++++-
>>>>>    1 file changed, 50 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>>>>> index 6f953e32d990..59fb415dfeeb 100644
>>>>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>>>>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>>>>> @@ -248,6 +248,8 @@ struct qcom_pcie_cfg {
>>>>>           bool no_l0s;
>>>>>    };
>>>>>
>>>>> +#define QCOM_PCIE_SLOT_MAX_SUPPLIES                    3
>>>>> +
>>>>>    struct qcom_pcie {
>>>>>           struct dw_pcie *pci;
>>>>>           void __iomem *parf;                     /* DT parf */
>>>>> @@ -260,6 +262,7 @@ struct qcom_pcie {
>>>>>           struct icc_path *icc_cpu;
>>>>>           const struct qcom_pcie_cfg *cfg;
>>>>>           struct dentry *debugfs;
>>>>> +       struct regulator_bulk_data slot_supplies[QCOM_PCIE_SLOT_MAX_SUPPLIES];
>>>>>           bool suspended;
>>>>>           bool use_pm_opp;
>>>>>    };
>>>>> @@ -1174,6 +1177,41 @@ static int qcom_pcie_link_up(struct dw_pcie *pci)
>>>>>           return !!(val & PCI_EXP_LNKSTA_DLLLA);
>>>>>    }
>>>>>
>>>>> +static int qcom_pcie_enable_slot_supplies(struct qcom_pcie *pcie)
>>>>> +{
>>>>> +       struct dw_pcie *pci = pcie->pci;
>>>>> +       int ret;
>>>>> +
>>>>> +       ret = regulator_bulk_enable(ARRAY_SIZE(pcie->slot_supplies),
>>>>> +                                   pcie->slot_supplies);
>>>>> +       if (ret < 0)
>>>>> +               dev_err(pci->dev, "Failed to enable slot regulators\n");
>>>>> +
>>>>> +       return ret;
>>>>> +}
>>>>> +
>>>>> +static void qcom_pcie_disable_slot_supplies(struct qcom_pcie *pcie)
>>>>> +{
>>>>> +       regulator_bulk_disable(ARRAY_SIZE(pcie->slot_supplies),
>>>>> +                              pcie->slot_supplies);
>>>>> +}
>>>>> +
>>>>> +static int qcom_pcie_get_slot_supplies(struct qcom_pcie *pcie)
>>>>> +{
>>>>> +       struct dw_pcie *pci = pcie->pci;
>>>>> +       int ret;
>>>>> +
>>>>> +       pcie->slot_supplies[0].supply = "vpcie12v";
>>>>> +       pcie->slot_supplies[1].supply = "vpcie3v3";
>>>>> +       pcie->slot_supplies[2].supply = "vpcie3v3aux";
>>>>> +       ret = devm_regulator_bulk_get(pci->dev, ARRAY_SIZE(pcie->slot_supplies),
>>>>> +                                     pcie->slot_supplies);
>>>>> +       if (ret < 0)
>>>>> +               dev_err(pci->dev, "Failed to get slot regulators\n");
>>>>> +
>>>>> +       return ret;
>>>>> +}
>>>>> +
>>>>>    static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
>>>>>    {
>>>>>           struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>>>>> @@ -1182,10 +1220,14 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
>>>>>
>>>>>           qcom_ep_reset_assert(pcie);
>>>>>
>>>>> -       ret = pcie->cfg->ops->init(pcie);
>>>>> +       ret = qcom_pcie_enable_slot_supplies(pcie);
>>>>>           if (ret)
>>>>>                   return ret;
>>>>>
>>>>> +       ret = pcie->cfg->ops->init(pcie);
>>>>> +       if (ret)
>>>>> +               goto err_disable_slot;
>>>>> +
>>>>>           ret = phy_set_mode_ext(pcie->phy, PHY_MODE_PCIE, PHY_MODE_PCIE_RC);
>>>>>           if (ret)
>>>>>                   goto err_deinit;
>>>>> @@ -1216,7 +1258,8 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
>>>>>           phy_power_off(pcie->phy);
>>>>>    err_deinit:
>>>>>           pcie->cfg->ops->deinit(pcie);
>>>>> -
>>>>> +err_disable_slot:
>>>>> +       qcom_pcie_disable_slot_supplies(pcie);
>>>>>           return ret;
>>>>>    }
>>>>>
>>>>> @@ -1228,6 +1271,7 @@ static void qcom_pcie_host_deinit(struct dw_pcie_rp *pp)
>>>>>           qcom_ep_reset_assert(pcie);
>>>>>           phy_power_off(pcie->phy);
>>>>>           pcie->cfg->ops->deinit(pcie);
>>>>> +       qcom_pcie_disable_slot_supplies(pcie);
>>>>>    }
>>>>>
>>>>>    static void qcom_pcie_host_post_init(struct dw_pcie_rp *pp)
>>>>> @@ -1602,6 +1646,10 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>>>>>                           goto err_pm_runtime_put;
>>>>>           }
>>>>>
>>>>> +       ret = qcom_pcie_get_slot_supplies(pcie);
>>>>> +       if (ret)
>>>>> +               goto err_pm_runtime_put;
>>>>> +
>>>>>           ret = pcie->cfg->ops->get_resources(pcie);
>>>>>           if (ret)
>>>>>                   goto err_pm_runtime_put;
>>>>> --
>>>>> 2.34.1
>>>>>
>>>> -- 
>>>> With best wishes
>>>> Dmitry

Re: [PATCH 8/8] PCI: qcom: Add support to PCIe slot power supplies

[Konrad Dybcio]

On 12.09.2024 3:39 PM, Qiang Yu wrote:
> 
> On 9/11/2024 11:32 PM, Manivannan Sadhasivam wrote:
>> On Wed, Sep 11, 2024 at 04:17:41PM +0800, Qiang Yu wrote:
>>> On 8/28/2024 12:58 AM, Manivannan Sadhasivam wrote:
>>>> On Tue, Aug 27, 2024 at 02:44:09PM +0300, Dmitry Baryshkov wrote:
>>>>> On Tue, 27 Aug 2024 at 09:36, Qiang Yu <quic_qianyu@quicinc.com> wrote:
>>>>>> On platform x1e80100 QCP, PCIe3 is a standard x8 form factor. Hence, add
>>>>>> support to use 3.3v, 3.3v aux and 12v regulators.
>>>>> First of all, I don't see corresponding bindings change.
>>>>>
>>>>> Second, these supplies power up the slot, not the host controller
>>>>> itself. As such these supplies do not belong to the host controller
>>>>> entry. Please consider using the pwrseq framework instead.
>>>>>
>>>> Indeed. For legacy reasons, slot power supplies were populated in the host
>>>> bridge node itself until recently Rob started objecting it [1]. And it makes
>>>> real sense to put these supplies in the root port node and handle them in the
>>>> relevant driver.
>>>>
>>>> I'm still evaluating whether the handling should be done in the portdrv or
>>>> pwrctl driver, but haven't reached the conclusion. Pwrctl seems to be the ideal
>>>> choice, but I see a few issues related to handling the OF node for the root
>>>> port.
>>>>
>>>> Hope I'll come to a conclusion in the next few days and will update this thread.
>>>>
>>>> - Mani
>>>>
>>>> [1] https://lore.kernel.org/lkml/20240604235806.GA1903493-robh@kernel.org/
>>> Hi Mani, do you have any updates?
>>>
>> I'm working with Bartosz to add a new pwrctl driver for rootports. And we are
>> debugging an issue currently. Unfortunately, the progress is very slow as I'm on
>> vacation still.
>>
>> Will post the patches once it got resolved.
>>
>> - Mani
> OK, thanks for your update.

Qiang, you can still resubmit the rest of the patches without having
to wait on that to be resolved

Konrad

Re: [PATCH 8/8] PCI: qcom: Add support to PCIe slot power supplies

[Manivannan Sadhasivam]

On Thu, Sep 12, 2024 at 04:15:56PM +0200, Konrad Dybcio wrote:
> On 12.09.2024 3:39 PM, Qiang Yu wrote:
> > 
> > On 9/11/2024 11:32 PM, Manivannan Sadhasivam wrote:
> >> On Wed, Sep 11, 2024 at 04:17:41PM +0800, Qiang Yu wrote:
> >>> On 8/28/2024 12:58 AM, Manivannan Sadhasivam wrote:
> >>>> On Tue, Aug 27, 2024 at 02:44:09PM +0300, Dmitry Baryshkov wrote:
> >>>>> On Tue, 27 Aug 2024 at 09:36, Qiang Yu <quic_qianyu@quicinc.com> wrote:
> >>>>>> On platform x1e80100 QCP, PCIe3 is a standard x8 form factor. Hence, add
> >>>>>> support to use 3.3v, 3.3v aux and 12v regulators.
> >>>>> First of all, I don't see corresponding bindings change.
> >>>>>
> >>>>> Second, these supplies power up the slot, not the host controller
> >>>>> itself. As such these supplies do not belong to the host controller
> >>>>> entry. Please consider using the pwrseq framework instead.
> >>>>>
> >>>> Indeed. For legacy reasons, slot power supplies were populated in the host
> >>>> bridge node itself until recently Rob started objecting it [1]. And it makes
> >>>> real sense to put these supplies in the root port node and handle them in the
> >>>> relevant driver.
> >>>>
> >>>> I'm still evaluating whether the handling should be done in the portdrv or
> >>>> pwrctl driver, but haven't reached the conclusion. Pwrctl seems to be the ideal
> >>>> choice, but I see a few issues related to handling the OF node for the root
> >>>> port.
> >>>>
> >>>> Hope I'll come to a conclusion in the next few days and will update this thread.
> >>>>
> >>>> - Mani
> >>>>
> >>>> [1] https://lore.kernel.org/lkml/20240604235806.GA1903493-robh@kernel.org/
> >>> Hi Mani, do you have any updates?
> >>>
> >> I'm working with Bartosz to add a new pwrctl driver for rootports. And we are
> >> debugging an issue currently. Unfortunately, the progress is very slow as I'm on
> >> vacation still.
> >>
> >> Will post the patches once it got resolved.
> >>
> >> - Mani
> > OK, thanks for your update.
> 
> Qiang, you can still resubmit the rest of the patches without having
> to wait on that to be resolved
> 

In that case, the slot supplies should be described in the PCIe bridge.

- Mani

> Konrad

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 8/8] PCI: qcom: Add support to PCIe slot power supplies

[Dmitry Baryshkov]

On Thu, 12 Sept 2024 at 17:45, Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> On Thu, Sep 12, 2024 at 04:15:56PM +0200, Konrad Dybcio wrote:
> > On 12.09.2024 3:39 PM, Qiang Yu wrote:
> > >
> > > On 9/11/2024 11:32 PM, Manivannan Sadhasivam wrote:
> > >> On Wed, Sep 11, 2024 at 04:17:41PM +0800, Qiang Yu wrote:
> > >>> On 8/28/2024 12:58 AM, Manivannan Sadhasivam wrote:
> > >>>> On Tue, Aug 27, 2024 at 02:44:09PM +0300, Dmitry Baryshkov wrote:
> > >>>>> On Tue, 27 Aug 2024 at 09:36, Qiang Yu <quic_qianyu@quicinc.com> wrote:
> > >>>>>> On platform x1e80100 QCP, PCIe3 is a standard x8 form factor. Hence, add
> > >>>>>> support to use 3.3v, 3.3v aux and 12v regulators.
> > >>>>> First of all, I don't see corresponding bindings change.
> > >>>>>
> > >>>>> Second, these supplies power up the slot, not the host controller
> > >>>>> itself. As such these supplies do not belong to the host controller
> > >>>>> entry. Please consider using the pwrseq framework instead.
> > >>>>>
> > >>>> Indeed. For legacy reasons, slot power supplies were populated in the host
> > >>>> bridge node itself until recently Rob started objecting it [1]. And it makes
> > >>>> real sense to put these supplies in the root port node and handle them in the
> > >>>> relevant driver.
> > >>>>
> > >>>> I'm still evaluating whether the handling should be done in the portdrv or
> > >>>> pwrctl driver, but haven't reached the conclusion. Pwrctl seems to be the ideal
> > >>>> choice, but I see a few issues related to handling the OF node for the root
> > >>>> port.
> > >>>>
> > >>>> Hope I'll come to a conclusion in the next few days and will update this thread.
> > >>>>
> > >>>> - Mani
> > >>>>
> > >>>> [1] https://lore.kernel.org/lkml/20240604235806.GA1903493-robh@kernel.org/
> > >>> Hi Mani, do you have any updates?
> > >>>
> > >> I'm working with Bartosz to add a new pwrctl driver for rootports. And we are
> > >> debugging an issue currently. Unfortunately, the progress is very slow as I'm on
> > >> vacation still.
> > >>
> > >> Will post the patches once it got resolved.
> > >>
> > >> - Mani
> > > OK, thanks for your update.
> >
> > Qiang, you can still resubmit the rest of the patches without having
> > to wait on that to be resolved
> >
>
> In that case, the slot supplies should be described in the PCIe bridge.

Patches 1-6 don't seem to depend on slot supplies, so they can be
submitted separately.

>
> - Mani
>
> > Konrad
>
> --
> மணிவண்ணன் சதாசிவம்



-- 
With best wishes
Dmitry

Re: [PATCH 8/8] PCI: qcom: Add support to PCIe slot power supplies

[Qiang Yu]

On 9/12/2024 10:49 PM, Dmitry Baryshkov wrote:
> On Thu, 12 Sept 2024 at 17:45, Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
>> On Thu, Sep 12, 2024 at 04:15:56PM +0200, Konrad Dybcio wrote:
>>> On 12.09.2024 3:39 PM, Qiang Yu wrote:
>>>> On 9/11/2024 11:32 PM, Manivannan Sadhasivam wrote:
>>>>> On Wed, Sep 11, 2024 at 04:17:41PM +0800, Qiang Yu wrote:
>>>>>> On 8/28/2024 12:58 AM, Manivannan Sadhasivam wrote:
>>>>>>> On Tue, Aug 27, 2024 at 02:44:09PM +0300, Dmitry Baryshkov wrote:
>>>>>>>> On Tue, 27 Aug 2024 at 09:36, Qiang Yu <quic_qianyu@quicinc.com> wrote:
>>>>>>>>> On platform x1e80100 QCP, PCIe3 is a standard x8 form factor. Hence, add
>>>>>>>>> support to use 3.3v, 3.3v aux and 12v regulators.
>>>>>>>> First of all, I don't see corresponding bindings change.
>>>>>>>>
>>>>>>>> Second, these supplies power up the slot, not the host controller
>>>>>>>> itself. As such these supplies do not belong to the host controller
>>>>>>>> entry. Please consider using the pwrseq framework instead.
>>>>>>>>
>>>>>>> Indeed. For legacy reasons, slot power supplies were populated in the host
>>>>>>> bridge node itself until recently Rob started objecting it [1]. And it makes
>>>>>>> real sense to put these supplies in the root port node and handle them in the
>>>>>>> relevant driver.
>>>>>>>
>>>>>>> I'm still evaluating whether the handling should be done in the portdrv or
>>>>>>> pwrctl driver, but haven't reached the conclusion. Pwrctl seems to be the ideal
>>>>>>> choice, but I see a few issues related to handling the OF node for the root
>>>>>>> port.
>>>>>>>
>>>>>>> Hope I'll come to a conclusion in the next few days and will update this thread.
>>>>>>>
>>>>>>> - Mani
>>>>>>>
>>>>>>> [1] https://lore.kernel.org/lkml/20240604235806.GA1903493-robh@kernel.org/
>>>>>> Hi Mani, do you have any updates?
>>>>>>
>>>>> I'm working with Bartosz to add a new pwrctl driver for rootports. And we are
>>>>> debugging an issue currently. Unfortunately, the progress is very slow as I'm on
>>>>> vacation still.
>>>>>
>>>>> Will post the patches once it got resolved.
>>>>>
>>>>> - Mani
>>>> OK, thanks for your update.
>>> Qiang, you can still resubmit the rest of the patches without having
>>> to wait on that to be resolved
>>>
>> In that case, the slot supplies should be described in the PCIe bridge.
> Patches 1-6 don't seem to depend on slot supplies, so they can be
> submitted separately.
OK, let me send v2 patch. Hi Mani, if you need any supports, please let 
me know.

Thanks,
Qiang
>
>> - Mani
>>
>>> Konrad
>> --
>> மணிவண்ணன் சதாசிவம்
>
>

Re: [PATCH 8/8] PCI: qcom: Add support to PCIe slot power supplies

[Qiang Yu]

On 8/27/2024 7:44 PM, Dmitry Baryshkov wrote:
> On Tue, 27 Aug 2024 at 09:36, Qiang Yu <quic_qianyu@quicinc.com> wrote:
>> On platform x1e80100 QCP, PCIe3 is a standard x8 form factor. Hence, add
>> support to use 3.3v, 3.3v aux and 12v regulators.
> First of all, I don't see corresponding bindings change.
>
> Second, these supplies power up the slot, not the host controller
> itself. As such these supplies do not belong to the host controller
> entry. Please consider using the pwrseq framework instead.
As Mani commented, he is exploring to use pwrctl driver to control this
three power. Will update the patch after Mani share his conclusion. This
patch may even not required.

Thanks,
Qiang
>
>> Signed-off-by: Qiang Yu <quic_qianyu@quicinc.com>
>> ---
>>   drivers/pci/controller/dwc/pcie-qcom.c | 52 +++++++++++++++++++++++++-
>>   1 file changed, 50 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>> index 6f953e32d990..59fb415dfeeb 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -248,6 +248,8 @@ struct qcom_pcie_cfg {
>>          bool no_l0s;
>>   };
>>
>> +#define QCOM_PCIE_SLOT_MAX_SUPPLIES                    3
>> +
>>   struct qcom_pcie {
>>          struct dw_pcie *pci;
>>          void __iomem *parf;                     /* DT parf */
>> @@ -260,6 +262,7 @@ struct qcom_pcie {
>>          struct icc_path *icc_cpu;
>>          const struct qcom_pcie_cfg *cfg;
>>          struct dentry *debugfs;
>> +       struct regulator_bulk_data slot_supplies[QCOM_PCIE_SLOT_MAX_SUPPLIES];
>>          bool suspended;
>>          bool use_pm_opp;
>>   };
>> @@ -1174,6 +1177,41 @@ static int qcom_pcie_link_up(struct dw_pcie *pci)
>>          return !!(val & PCI_EXP_LNKSTA_DLLLA);
>>   }
>>
>> +static int qcom_pcie_enable_slot_supplies(struct qcom_pcie *pcie)
>> +{
>> +       struct dw_pcie *pci = pcie->pci;
>> +       int ret;
>> +
>> +       ret = regulator_bulk_enable(ARRAY_SIZE(pcie->slot_supplies),
>> +                                   pcie->slot_supplies);
>> +       if (ret < 0)
>> +               dev_err(pci->dev, "Failed to enable slot regulators\n");
>> +
>> +       return ret;
>> +}
>> +
>> +static void qcom_pcie_disable_slot_supplies(struct qcom_pcie *pcie)
>> +{
>> +       regulator_bulk_disable(ARRAY_SIZE(pcie->slot_supplies),
>> +                              pcie->slot_supplies);
>> +}
>> +
>> +static int qcom_pcie_get_slot_supplies(struct qcom_pcie *pcie)
>> +{
>> +       struct dw_pcie *pci = pcie->pci;
>> +       int ret;
>> +
>> +       pcie->slot_supplies[0].supply = "vpcie12v";
>> +       pcie->slot_supplies[1].supply = "vpcie3v3";
>> +       pcie->slot_supplies[2].supply = "vpcie3v3aux";
>> +       ret = devm_regulator_bulk_get(pci->dev, ARRAY_SIZE(pcie->slot_supplies),
>> +                                     pcie->slot_supplies);
>> +       if (ret < 0)
>> +               dev_err(pci->dev, "Failed to get slot regulators\n");
>> +
>> +       return ret;
>> +}
>> +
>>   static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
>>   {
>>          struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> @@ -1182,10 +1220,14 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
>>
>>          qcom_ep_reset_assert(pcie);
>>
>> -       ret = pcie->cfg->ops->init(pcie);
>> +       ret = qcom_pcie_enable_slot_supplies(pcie);
>>          if (ret)
>>                  return ret;
>>
>> +       ret = pcie->cfg->ops->init(pcie);
>> +       if (ret)
>> +               goto err_disable_slot;
>> +
>>          ret = phy_set_mode_ext(pcie->phy, PHY_MODE_PCIE, PHY_MODE_PCIE_RC);
>>          if (ret)
>>                  goto err_deinit;
>> @@ -1216,7 +1258,8 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
>>          phy_power_off(pcie->phy);
>>   err_deinit:
>>          pcie->cfg->ops->deinit(pcie);
>> -
>> +err_disable_slot:
>> +       qcom_pcie_disable_slot_supplies(pcie);
>>          return ret;
>>   }
>>
>> @@ -1228,6 +1271,7 @@ static void qcom_pcie_host_deinit(struct dw_pcie_rp *pp)
>>          qcom_ep_reset_assert(pcie);
>>          phy_power_off(pcie->phy);
>>          pcie->cfg->ops->deinit(pcie);
>> +       qcom_pcie_disable_slot_supplies(pcie);
>>   }
>>
>>   static void qcom_pcie_host_post_init(struct dw_pcie_rp *pp)
>> @@ -1602,6 +1646,10 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>>                          goto err_pm_runtime_put;
>>          }
>>
>> +       ret = qcom_pcie_get_slot_supplies(pcie);
>> +       if (ret)
>> +               goto err_pm_runtime_put;
>> +
>>          ret = pcie->cfg->ops->get_resources(pcie);
>>          if (ret)
>>                  goto err_pm_runtime_put;
>> --
>> 2.34.1
>>
>

Re: [PATCH 0/8] Add support for PCIe3 on x1e80100

[Rob Herring (Arm)]

On Mon, 26 Aug 2024 23:36:23 -0700, Qiang Yu wrote:
> This series add support for PCIe3 on x1e80100.
> 
> PCIe3 needs additional set of clocks, regulators and new set of PCIe QMP
> PHY configuration compare other PCIe instances on x1e80100. Hence add
> required resource configuration and usage for PCIe3.
> 
> Qiang Yu (8):
>   phy: qcom-qmp: pcs-pcie: Add v6.30 register offsets
>   phy: qcom-qmp: pcs: Add v6.30 register offsets
>   phy: qcom: qmp: Add phy register and clk setting for x1e80100 PCIe3
>   arm64: dts: qcom: x1e80100: Add support for PCIe3 on x1e80100
>   dt-bindings: phy: qcom,sc8280xp-qmp-pcie-phy: Document the X1E80100
>     QMP PCIe PHY Gen4 x8
>   clk: qcom: gcc-x1e80100: Fix halt_check for pipediv2 clocks
>   arm64: dts: qcom: x1e80100-qcp: Add power supply and sideband signal
>     for pcie3
>   PCI: qcom: Add support to PCIe slot power supplies
> 
>  .../phy/qcom,sc8280xp-qmp-pcie-phy.yaml       |  18 +-
>  arch/arm64/boot/dts/qcom/x1e80100-qcp.dts     | 116 +++++++++
>  arch/arm64/boot/dts/qcom/x1e80100.dtsi        | 205 +++++++++++++++-
>  drivers/clk/qcom/gcc-x1e80100.c               |  10 +-
>  drivers/pci/controller/dwc/pcie-qcom.c        |  52 +++-
>  drivers/phy/qualcomm/phy-qcom-qmp-pcie.c      | 222 +++++++++++++++++-
>  .../qualcomm/phy-qcom-qmp-pcs-pcie-v6_30.h    |  25 ++
>  drivers/phy/qualcomm/phy-qcom-qmp-pcs-v6_30.h |  19 ++
>  8 files changed, 657 insertions(+), 10 deletions(-)
>  create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-pcs-pcie-v6_30.h
>  create mode 100644 drivers/phy/qualcomm/phy-qcom-qmp-pcs-v6_30.h
> 
> --
> 2.34.1
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


New warnings running 'make CHECK_DTBS=y qcom/x1e80100-qcp.dtb' for 20240827063631.3932971-1-quic_qianyu@quicinc.com:

arch/arm64/boot/dts/qcom/x1e80100-qcp.dtb: pci@1bd0000: reg: [[0, 29163520, 0, 12288], [0, 2013265920, 0, 3869], [0, 2013269824, 0, 168], [0, 2013270016, 0, 4096], [0, 2014314496, 0, 1048576]] is too short
	from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-x1e80100.yaml#
arch/arm64/boot/dts/qcom/x1e80100-qcp.dtb: pci@1bd0000: reg-names: ['parf', 'dbi', 'elbi', 'atu', 'config'] is too short
	from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-x1e80100.yaml#
arch/arm64/boot/dts/qcom/x1e80100-qcp.dtb: pci@1bd0000: clocks: [[53, 348], [53, 84], [53, 86], [53, 87], [53, 94], [53, 95], [53, 22], [53, 33]] is too long
	from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-x1e80100.yaml#
arch/arm64/boot/dts/qcom/x1e80100-qcp.dtb: pci@1bd0000: clock-names:0: 'aux' was expected
	from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-x1e80100.yaml#
arch/arm64/boot/dts/qcom/x1e80100-qcp.dtb: pci@1bd0000: clock-names:1: 'cfg' was expected
	from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-x1e80100.yaml#
arch/arm64/boot/dts/qcom/x1e80100-qcp.dtb: pci@1bd0000: clock-names:2: 'bus_master' was expected
	from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-x1e80100.yaml#
arch/arm64/boot/dts/qcom/x1e80100-qcp.dtb: pci@1bd0000: clock-names:3: 'bus_slave' was expected
	from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-x1e80100.yaml#
arch/arm64/boot/dts/qcom/x1e80100-qcp.dtb: pci@1bd0000: clock-names:4: 'slave_q2a' was expected
	from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-x1e80100.yaml#
arch/arm64/boot/dts/qcom/x1e80100-qcp.dtb: pci@1bd0000: clock-names:5: 'noc_aggr' was expected
	from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-x1e80100.yaml#
arch/arm64/boot/dts/qcom/x1e80100-qcp.dtb: pci@1bd0000: clock-names:6: 'cnoc_sf_axi' was expected
	from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-x1e80100.yaml#
arch/arm64/boot/dts/qcom/x1e80100-qcp.dtb: pci@1bd0000: clock-names: ['pipe_clk_src', 'aux', 'cfg', 'bus_master', 'bus_slave', 'slave_q2a', 'noc_aggr', 'cnoc_sf_axi'] is too long
	from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-x1e80100.yaml#
arch/arm64/boot/dts/qcom/x1e80100-qcp.dtb: pci@1bd0000: Unevaluated properties are not allowed ('operating-points-v2', 'opp-table' were unexpected)
	from schema $id: http://devicetree.org/schemas/pci/qcom,pcie-x1e80100.yaml#
arch/arm64/boot/dts/qcom/x1e80100-qcp.dtb: phy@1be0000: clock-names:4: 'pipe' was expected
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-pcie-phy.yaml#
arch/arm64/boot/dts/qcom/x1e80100-qcp.dtb: phy@1be0000: clock-names:5: 'pipediv2' was expected
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-pcie-phy.yaml#
arch/arm64/boot/dts/qcom/x1e80100-qcp.dtb: phy@1be0000: clock-names:6: 'phy_aux' was expected
	from schema $id: http://devicetree.org/schemas/phy/qcom,sc8280xp-qmp-pcie-phy.yaml#
arch/arm64/boot/dts/qcom/x1e80100-qcp.dtb: pmic@3: gpio@8800: 'pm_sde7_aux_3p3', 'pm_sde7_main_3p3' do not match any of the regexes: '-hog(-[0-9]+)?$', '-state$', 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/mfd/qcom,spmi-pmic.yaml#
arch/arm64/boot/dts/qcom/x1e80100-qcp.dtb: gpio@8800: 'pm_sde7_aux_3p3', 'pm_sde7_main_3p3' do not match any of the regexes: '-hog(-[0-9]+)?$', '-state$', 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/pinctrl/qcom,pmic-gpio.yaml#
arch/arm64/boot/dts/qcom/x1e80100-qcp.dtb: pmic@8: gpio@8800: 'pcie_x8_12v_on' does not match any of the regexes: '-hog(-[0-9]+)?$', '-state$', 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/mfd/qcom,spmi-pmic.yaml#
arch/arm64/boot/dts/qcom/x1e80100-qcp.dtb: gpio@8800: 'pcie_x8_12v_on' does not match any of the regexes: '-hog(-[0-9]+)?$', '-state$', 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/pinctrl/qcom,pmic-gpio.yaml#