[RFC PATCH 0/4] PCI: qcom: support using the same PHY for both RC and EP

[RFC PATCH 0/4] PCI: qcom: support using the same PHY for both RC and EP

[Dmitry Baryshkov]

Programming of QMP PCIe PHYs slightly differs between RC and EP modes.

Currently both qcom and qcom-ep PCIe controllers setup the PHY in the
default mode, making it impossible to select at runtime whether the PHY
should be running in RC or in EP modes. Usually this is not an issue,
since for most devices only the RC mode is used, while for some (SDX55)
the EP mode is used without support for working as the RC.

Some of the Qualcomm platforms would still benefit from being able to
switch between RC and EP depending on the driver being used. While it is
possible to use different compat strings for the PHY depending on the
mode, it seems like an incorrect approach, since the PHY doesn't differ
between usecases. It's the PCIe controller, who should decide how to
configure the PHY.

This patch series implements the ability to select between RC and EP
modes, by allowing the PCIe QMP PHY driver to switch between
programming tables.

Dmitry Baryshkov (4):
  phy: qcom-qmp-pcie: split register tables into primary and secondary
    part
  phy: qcom-qmp-pcie: suppor separate tables for EP mode
  PCI: qcom: call phy_set_mode_ext()
  PCI: qcom-ep: call phy_set_mode_ext()

 drivers/pci/controller/dwc/pcie-qcom-ep.c |   4 +
 drivers/pci/controller/dwc/pcie-qcom.c    |   4 +
 drivers/phy/qualcomm/phy-qcom-qmp-pcie.c  | 155 ++++++++++++----------
 3 files changed, 96 insertions(+), 67 deletions(-)

-- 
2.35.1

[RFC PATCH 1/4] phy: qcom-qmp-pcie: split register tables into primary and secondary part

[Dmitry Baryshkov]

Split register tables list into primary and secondary parts. While we
are at it, drop unused if (table) conditions, since the function
qcom_qmp_phy_pcie_configure_lane() has this check anyway.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 144 ++++++++++++-----------
 1 file changed, 77 insertions(+), 67 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
index 2d65e1f56bfc..23ca5848c4a8 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
@@ -1346,34 +1346,29 @@ static const struct qmp_phy_init_tbl sm8450_qmp_gen4x2_pcie_pcs_misc_tbl[] = {
 
 struct qmp_phy;
 
-/* struct qmp_phy_cfg - per-PHY initialization config */
-struct qmp_phy_cfg {
-	/* phy-type - PCIE/UFS/USB */
-	unsigned int type;
-	/* number of lanes provided by phy */
-	int nlanes;
-
-	/* Init sequence for PHY blocks - serdes, tx, rx, pcs */
+struct qmp_phy_cfg_tables {
 	const struct qmp_phy_init_tbl *serdes_tbl;
 	int serdes_tbl_num;
-	const struct qmp_phy_init_tbl *serdes_tbl_sec;
-	int serdes_tbl_num_sec;
 	const struct qmp_phy_init_tbl *tx_tbl;
 	int tx_tbl_num;
-	const struct qmp_phy_init_tbl *tx_tbl_sec;
-	int tx_tbl_num_sec;
 	const struct qmp_phy_init_tbl *rx_tbl;
 	int rx_tbl_num;
-	const struct qmp_phy_init_tbl *rx_tbl_sec;
-	int rx_tbl_num_sec;
 	const struct qmp_phy_init_tbl *pcs_tbl;
 	int pcs_tbl_num;
-	const struct qmp_phy_init_tbl *pcs_tbl_sec;
-	int pcs_tbl_num_sec;
 	const struct qmp_phy_init_tbl *pcs_misc_tbl;
 	int pcs_misc_tbl_num;
-	const struct qmp_phy_init_tbl *pcs_misc_tbl_sec;
-	int pcs_misc_tbl_num_sec;
+};
+
+/* struct qmp_phy_cfg - per-PHY initialization config */
+struct qmp_phy_cfg {
+	/* phy-type - PCIE/UFS/USB */
+	unsigned int type;
+	/* number of lanes provided by phy */
+	int nlanes;
+
+	/* Init sequence for PHY blocks - serdes, tx, rx, pcs */
+	struct qmp_phy_cfg_tables pri;
+	struct qmp_phy_cfg_tables sec;
 
 	/* clock ids to be requested */
 	const char * const *clk_list;
@@ -1517,6 +1512,7 @@ static const struct qmp_phy_cfg ipq8074_pciephy_cfg = {
 	.type			= PHY_TYPE_PCIE,
 	.nlanes			= 1,
 
+	.pri = {
 	.serdes_tbl		= ipq8074_pcie_serdes_tbl,
 	.serdes_tbl_num		= ARRAY_SIZE(ipq8074_pcie_serdes_tbl),
 	.tx_tbl			= ipq8074_pcie_tx_tbl,
@@ -1525,6 +1521,7 @@ static const struct qmp_phy_cfg ipq8074_pciephy_cfg = {
 	.rx_tbl_num		= ARRAY_SIZE(ipq8074_pcie_rx_tbl),
 	.pcs_tbl		= ipq8074_pcie_pcs_tbl,
 	.pcs_tbl_num		= ARRAY_SIZE(ipq8074_pcie_pcs_tbl),
+	},
 	.clk_list		= ipq8074_pciephy_clk_l,
 	.num_clks		= ARRAY_SIZE(ipq8074_pciephy_clk_l),
 	.reset_list		= ipq8074_pciephy_reset_l,
@@ -1546,6 +1543,7 @@ static const struct qmp_phy_cfg ipq8074_pciephy_gen3_cfg = {
 	.type			= PHY_TYPE_PCIE,
 	.nlanes			= 1,
 
+	.pri = {
 	.serdes_tbl		= ipq8074_pcie_gen3_serdes_tbl,
 	.serdes_tbl_num		= ARRAY_SIZE(ipq8074_pcie_gen3_serdes_tbl),
 	.tx_tbl			= ipq8074_pcie_gen3_tx_tbl,
@@ -1554,6 +1552,7 @@ static const struct qmp_phy_cfg ipq8074_pciephy_gen3_cfg = {
 	.rx_tbl_num		= ARRAY_SIZE(ipq8074_pcie_gen3_rx_tbl),
 	.pcs_tbl		= ipq8074_pcie_gen3_pcs_tbl,
 	.pcs_tbl_num		= ARRAY_SIZE(ipq8074_pcie_gen3_pcs_tbl),
+	},
 	.clk_list		= ipq8074_pciephy_clk_l,
 	.num_clks		= ARRAY_SIZE(ipq8074_pciephy_clk_l),
 	.reset_list		= ipq8074_pciephy_reset_l,
@@ -1576,6 +1575,7 @@ static const struct qmp_phy_cfg ipq6018_pciephy_cfg = {
 	.type			= PHY_TYPE_PCIE,
 	.nlanes			= 1,
 
+	.pri = {
 	.serdes_tbl		= ipq6018_pcie_serdes_tbl,
 	.serdes_tbl_num		= ARRAY_SIZE(ipq6018_pcie_serdes_tbl),
 	.tx_tbl			= ipq6018_pcie_tx_tbl,
@@ -1586,6 +1586,7 @@ static const struct qmp_phy_cfg ipq6018_pciephy_cfg = {
 	.pcs_tbl_num		= ARRAY_SIZE(ipq6018_pcie_pcs_tbl),
 	.pcs_misc_tbl		= ipq6018_pcie_pcs_misc_tbl,
 	.pcs_misc_tbl_num	= ARRAY_SIZE(ipq6018_pcie_pcs_misc_tbl),
+	},
 	.clk_list		= ipq8074_pciephy_clk_l,
 	.num_clks		= ARRAY_SIZE(ipq8074_pciephy_clk_l),
 	.reset_list		= ipq8074_pciephy_reset_l,
@@ -1606,6 +1607,7 @@ static const struct qmp_phy_cfg sdm845_qmp_pciephy_cfg = {
 	.type = PHY_TYPE_PCIE,
 	.nlanes = 1,
 
+	.pri = {
 	.serdes_tbl		= sdm845_qmp_pcie_serdes_tbl,
 	.serdes_tbl_num		= ARRAY_SIZE(sdm845_qmp_pcie_serdes_tbl),
 	.tx_tbl			= sdm845_qmp_pcie_tx_tbl,
@@ -1616,6 +1618,7 @@ static const struct qmp_phy_cfg sdm845_qmp_pciephy_cfg = {
 	.pcs_tbl_num		= ARRAY_SIZE(sdm845_qmp_pcie_pcs_tbl),
 	.pcs_misc_tbl		= sdm845_qmp_pcie_pcs_misc_tbl,
 	.pcs_misc_tbl_num	= ARRAY_SIZE(sdm845_qmp_pcie_pcs_misc_tbl),
+	},
 	.clk_list		= sdm845_pciephy_clk_l,
 	.num_clks		= ARRAY_SIZE(sdm845_pciephy_clk_l),
 	.reset_list		= sdm845_pciephy_reset_l,
@@ -1637,6 +1640,7 @@ static const struct qmp_phy_cfg sdm845_qhp_pciephy_cfg = {
 	.type = PHY_TYPE_PCIE,
 	.nlanes = 1,
 
+	.pri = {
 	.serdes_tbl		= sdm845_qhp_pcie_serdes_tbl,
 	.serdes_tbl_num		= ARRAY_SIZE(sdm845_qhp_pcie_serdes_tbl),
 	.tx_tbl			= sdm845_qhp_pcie_tx_tbl,
@@ -1645,6 +1649,7 @@ static const struct qmp_phy_cfg sdm845_qhp_pciephy_cfg = {
 	.rx_tbl_num		= ARRAY_SIZE(sdm845_qhp_pcie_rx_tbl),
 	.pcs_tbl		= sdm845_qhp_pcie_pcs_tbl,
 	.pcs_tbl_num		= ARRAY_SIZE(sdm845_qhp_pcie_pcs_tbl),
+	},
 	.clk_list		= sdm845_pciephy_clk_l,
 	.num_clks		= ARRAY_SIZE(sdm845_pciephy_clk_l),
 	.reset_list		= sdm845_pciephy_reset_l,
@@ -1666,24 +1671,28 @@ static const struct qmp_phy_cfg sm8250_qmp_gen3x1_pciephy_cfg = {
 	.type = PHY_TYPE_PCIE,
 	.nlanes = 1,
 
+	.pri = {
 	.serdes_tbl		= sm8250_qmp_pcie_serdes_tbl,
 	.serdes_tbl_num		= ARRAY_SIZE(sm8250_qmp_pcie_serdes_tbl),
-	.serdes_tbl_sec		= sm8250_qmp_gen3x1_pcie_serdes_tbl,
-	.serdes_tbl_num_sec	= ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_serdes_tbl),
 	.tx_tbl			= sm8250_qmp_pcie_tx_tbl,
 	.tx_tbl_num		= ARRAY_SIZE(sm8250_qmp_pcie_tx_tbl),
 	.rx_tbl			= sm8250_qmp_pcie_rx_tbl,
 	.rx_tbl_num		= ARRAY_SIZE(sm8250_qmp_pcie_rx_tbl),
-	.rx_tbl_sec		= sm8250_qmp_gen3x1_pcie_rx_tbl,
-	.rx_tbl_num_sec		= ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_rx_tbl),
 	.pcs_tbl		= sm8250_qmp_pcie_pcs_tbl,
 	.pcs_tbl_num		= ARRAY_SIZE(sm8250_qmp_pcie_pcs_tbl),
-	.pcs_tbl_sec		= sm8250_qmp_gen3x1_pcie_pcs_tbl,
-	.pcs_tbl_num_sec		= ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_pcs_tbl),
 	.pcs_misc_tbl		= sm8250_qmp_pcie_pcs_misc_tbl,
 	.pcs_misc_tbl_num	= ARRAY_SIZE(sm8250_qmp_pcie_pcs_misc_tbl),
-	.pcs_misc_tbl_sec		= sm8250_qmp_gen3x1_pcie_pcs_misc_tbl,
-	.pcs_misc_tbl_num_sec	= ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_pcs_misc_tbl),
+	},
+	.sec = {
+	.serdes_tbl		= sm8250_qmp_gen3x1_pcie_serdes_tbl,
+	.serdes_tbl_num		= ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_serdes_tbl),
+	.rx_tbl			= sm8250_qmp_gen3x1_pcie_rx_tbl,
+	.rx_tbl_num		= ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_rx_tbl),
+	.pcs_tbl		= sm8250_qmp_gen3x1_pcie_pcs_tbl,
+	.pcs_tbl_num		= ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_pcs_tbl),
+	.pcs_misc_tbl		= sm8250_qmp_gen3x1_pcie_pcs_misc_tbl,
+	.pcs_misc_tbl_num	= ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_pcs_misc_tbl),
+	},
 	.clk_list		= sdm845_pciephy_clk_l,
 	.num_clks		= ARRAY_SIZE(sdm845_pciephy_clk_l),
 	.reset_list		= sdm845_pciephy_reset_l,
@@ -1705,24 +1714,28 @@ static const struct qmp_phy_cfg sm8250_qmp_gen3x2_pciephy_cfg = {
 	.type = PHY_TYPE_PCIE,
 	.nlanes = 2,
 
+	.pri = {
 	.serdes_tbl		= sm8250_qmp_pcie_serdes_tbl,
 	.serdes_tbl_num		= ARRAY_SIZE(sm8250_qmp_pcie_serdes_tbl),
 	.tx_tbl			= sm8250_qmp_pcie_tx_tbl,
 	.tx_tbl_num		= ARRAY_SIZE(sm8250_qmp_pcie_tx_tbl),
-	.tx_tbl_sec		= sm8250_qmp_gen3x2_pcie_tx_tbl,
-	.tx_tbl_num_sec		= ARRAY_SIZE(sm8250_qmp_gen3x2_pcie_tx_tbl),
 	.rx_tbl			= sm8250_qmp_pcie_rx_tbl,
 	.rx_tbl_num		= ARRAY_SIZE(sm8250_qmp_pcie_rx_tbl),
-	.rx_tbl_sec		= sm8250_qmp_gen3x2_pcie_rx_tbl,
-	.rx_tbl_num_sec		= ARRAY_SIZE(sm8250_qmp_gen3x2_pcie_rx_tbl),
 	.pcs_tbl		= sm8250_qmp_pcie_pcs_tbl,
 	.pcs_tbl_num		= ARRAY_SIZE(sm8250_qmp_pcie_pcs_tbl),
-	.pcs_tbl_sec		= sm8250_qmp_gen3x2_pcie_pcs_tbl,
-	.pcs_tbl_num_sec		= ARRAY_SIZE(sm8250_qmp_gen3x2_pcie_pcs_tbl),
 	.pcs_misc_tbl		= sm8250_qmp_pcie_pcs_misc_tbl,
 	.pcs_misc_tbl_num	= ARRAY_SIZE(sm8250_qmp_pcie_pcs_misc_tbl),
-	.pcs_misc_tbl_sec		= sm8250_qmp_gen3x2_pcie_pcs_misc_tbl,
-	.pcs_misc_tbl_num_sec	= ARRAY_SIZE(sm8250_qmp_gen3x2_pcie_pcs_misc_tbl),
+	},
+	.sec = {
+	.tx_tbl			= sm8250_qmp_gen3x2_pcie_tx_tbl,
+	.tx_tbl_num		= ARRAY_SIZE(sm8250_qmp_gen3x2_pcie_tx_tbl),
+	.rx_tbl			= sm8250_qmp_gen3x2_pcie_rx_tbl,
+	.rx_tbl_num		= ARRAY_SIZE(sm8250_qmp_gen3x2_pcie_rx_tbl),
+	.pcs_tbl		= sm8250_qmp_gen3x2_pcie_pcs_tbl,
+	.pcs_tbl_num		= ARRAY_SIZE(sm8250_qmp_gen3x2_pcie_pcs_tbl),
+	.pcs_misc_tbl		= sm8250_qmp_gen3x2_pcie_pcs_misc_tbl,
+	.pcs_misc_tbl_num	= ARRAY_SIZE(sm8250_qmp_gen3x2_pcie_pcs_misc_tbl),
+	},
 	.clk_list		= sdm845_pciephy_clk_l,
 	.num_clks		= ARRAY_SIZE(sdm845_pciephy_clk_l),
 	.reset_list		= sdm845_pciephy_reset_l,
@@ -1745,6 +1758,7 @@ static const struct qmp_phy_cfg msm8998_pciephy_cfg = {
 	.type			= PHY_TYPE_PCIE,
 	.nlanes			= 1,
 
+	.pri = {
 	.serdes_tbl		= msm8998_pcie_serdes_tbl,
 	.serdes_tbl_num		= ARRAY_SIZE(msm8998_pcie_serdes_tbl),
 	.tx_tbl			= msm8998_pcie_tx_tbl,
@@ -1753,6 +1767,7 @@ static const struct qmp_phy_cfg msm8998_pciephy_cfg = {
 	.rx_tbl_num		= ARRAY_SIZE(msm8998_pcie_rx_tbl),
 	.pcs_tbl		= msm8998_pcie_pcs_tbl,
 	.pcs_tbl_num		= ARRAY_SIZE(msm8998_pcie_pcs_tbl),
+	},
 	.clk_list		= msm8996_phy_clk_l,
 	.num_clks		= ARRAY_SIZE(msm8996_phy_clk_l),
 	.reset_list		= ipq8074_pciephy_reset_l,
@@ -1770,6 +1785,7 @@ static const struct qmp_phy_cfg sc8180x_pciephy_cfg = {
 	.type = PHY_TYPE_PCIE,
 	.nlanes = 1,
 
+	.pri = {
 	.serdes_tbl		= sc8180x_qmp_pcie_serdes_tbl,
 	.serdes_tbl_num		= ARRAY_SIZE(sc8180x_qmp_pcie_serdes_tbl),
 	.tx_tbl			= sc8180x_qmp_pcie_tx_tbl,
@@ -1780,6 +1796,7 @@ static const struct qmp_phy_cfg sc8180x_pciephy_cfg = {
 	.pcs_tbl_num		= ARRAY_SIZE(sc8180x_qmp_pcie_pcs_tbl),
 	.pcs_misc_tbl		= sc8180x_qmp_pcie_pcs_misc_tbl,
 	.pcs_misc_tbl_num	= ARRAY_SIZE(sc8180x_qmp_pcie_pcs_misc_tbl),
+	},
 	.clk_list		= sdm845_pciephy_clk_l,
 	.num_clks		= ARRAY_SIZE(sdm845_pciephy_clk_l),
 	.reset_list		= sdm845_pciephy_reset_l,
@@ -1800,6 +1817,7 @@ static const struct qmp_phy_cfg sdx55_qmp_pciephy_cfg = {
 	.type = PHY_TYPE_PCIE,
 	.nlanes = 2,
 
+	.pri = {
 	.serdes_tbl		= sdx55_qmp_pcie_serdes_tbl,
 	.serdes_tbl_num		= ARRAY_SIZE(sdx55_qmp_pcie_serdes_tbl),
 	.tx_tbl			= sdx55_qmp_pcie_tx_tbl,
@@ -1810,6 +1828,7 @@ static const struct qmp_phy_cfg sdx55_qmp_pciephy_cfg = {
 	.pcs_tbl_num		= ARRAY_SIZE(sdx55_qmp_pcie_pcs_tbl),
 	.pcs_misc_tbl		= sdx55_qmp_pcie_pcs_misc_tbl,
 	.pcs_misc_tbl_num	= ARRAY_SIZE(sdx55_qmp_pcie_pcs_misc_tbl),
+	},
 	.clk_list		= sdm845_pciephy_clk_l,
 	.num_clks		= ARRAY_SIZE(sdm845_pciephy_clk_l),
 	.reset_list		= sdm845_pciephy_reset_l,
@@ -1832,6 +1851,7 @@ static const struct qmp_phy_cfg sm8450_qmp_gen3x1_pciephy_cfg = {
 	.type = PHY_TYPE_PCIE,
 	.nlanes = 1,
 
+	.pri = {
 	.serdes_tbl		= sm8450_qmp_gen3x1_pcie_serdes_tbl,
 	.serdes_tbl_num		= ARRAY_SIZE(sm8450_qmp_gen3x1_pcie_serdes_tbl),
 	.tx_tbl			= sm8450_qmp_gen3x1_pcie_tx_tbl,
@@ -1842,6 +1862,7 @@ static const struct qmp_phy_cfg sm8450_qmp_gen3x1_pciephy_cfg = {
 	.pcs_tbl_num		= ARRAY_SIZE(sm8450_qmp_gen3x1_pcie_pcs_tbl),
 	.pcs_misc_tbl		= sm8450_qmp_gen3x1_pcie_pcs_misc_tbl,
 	.pcs_misc_tbl_num	= ARRAY_SIZE(sm8450_qmp_gen3x1_pcie_pcs_misc_tbl),
+	},
 	.clk_list		= sdm845_pciephy_clk_l,
 	.num_clks		= ARRAY_SIZE(sdm845_pciephy_clk_l),
 	.reset_list		= sdm845_pciephy_reset_l,
@@ -1863,6 +1884,7 @@ static const struct qmp_phy_cfg sm8450_qmp_gen4x2_pciephy_cfg = {
 	.type = PHY_TYPE_PCIE,
 	.nlanes = 2,
 
+	.pri = {
 	.serdes_tbl		= sm8450_qmp_gen4x2_pcie_serdes_tbl,
 	.serdes_tbl_num		= ARRAY_SIZE(sm8450_qmp_gen4x2_pcie_serdes_tbl),
 	.tx_tbl			= sm8450_qmp_gen4x2_pcie_tx_tbl,
@@ -1873,6 +1895,7 @@ static const struct qmp_phy_cfg sm8450_qmp_gen4x2_pciephy_cfg = {
 	.pcs_tbl_num		= ARRAY_SIZE(sm8450_qmp_gen4x2_pcie_pcs_tbl),
 	.pcs_misc_tbl		= sm8450_qmp_gen4x2_pcie_pcs_misc_tbl,
 	.pcs_misc_tbl_num	= ARRAY_SIZE(sm8450_qmp_gen4x2_pcie_pcs_misc_tbl),
+	},
 	.clk_list		= sdm845_pciephy_clk_l,
 	.num_clks		= ARRAY_SIZE(sdm845_pciephy_clk_l),
 	.reset_list		= sdm845_pciephy_reset_l,
@@ -1926,13 +1949,9 @@ static int qcom_qmp_phy_pcie_serdes_init(struct qmp_phy *qphy)
 {
 	const struct qmp_phy_cfg *cfg = qphy->cfg;
 	void __iomem *serdes = qphy->serdes;
-	const struct qmp_phy_init_tbl *serdes_tbl = cfg->serdes_tbl;
-	int serdes_tbl_num = cfg->serdes_tbl_num;
 
-	qcom_qmp_phy_pcie_configure(serdes, cfg->regs, serdes_tbl, serdes_tbl_num);
-	if (cfg->serdes_tbl_sec)
-		qcom_qmp_phy_pcie_configure(serdes, cfg->regs, cfg->serdes_tbl_sec,
-				       cfg->serdes_tbl_num_sec);
+	qcom_qmp_phy_pcie_configure(serdes, cfg->regs, cfg->pri.serdes_tbl, cfg->pri.serdes_tbl_num);
+	qcom_qmp_phy_pcie_configure(serdes, cfg->regs, cfg->sec.serdes_tbl, cfg->sec.serdes_tbl_num);
 
 	return 0;
 }
@@ -2036,46 +2055,37 @@ static int qcom_qmp_phy_pcie_power_on(struct phy *phy)
 
 	/* Tx, Rx, and PCS configurations */
 	qcom_qmp_phy_pcie_configure_lane(tx, cfg->regs,
-				    cfg->tx_tbl, cfg->tx_tbl_num, 1);
-	if (cfg->tx_tbl_sec)
-		qcom_qmp_phy_pcie_configure_lane(tx, cfg->regs, cfg->tx_tbl_sec,
-					    cfg->tx_tbl_num_sec, 1);
+					 cfg->pri.tx_tbl, cfg->pri.tx_tbl_num, 1);
+	qcom_qmp_phy_pcie_configure_lane(tx, cfg->regs,
+					 cfg->sec.tx_tbl, cfg->sec.tx_tbl_num, 1);
 
 	/* Configuration for other LANE for USB-DP combo PHY */
 	if (cfg->is_dual_lane_phy) {
 		qcom_qmp_phy_pcie_configure_lane(qphy->tx2, cfg->regs,
-					    cfg->tx_tbl, cfg->tx_tbl_num, 2);
-		if (cfg->tx_tbl_sec)
-			qcom_qmp_phy_pcie_configure_lane(qphy->tx2, cfg->regs,
-						    cfg->tx_tbl_sec,
-						    cfg->tx_tbl_num_sec, 2);
+						 cfg->pri.tx_tbl, cfg->pri.tx_tbl_num, 2);
+		qcom_qmp_phy_pcie_configure_lane(qphy->tx2, cfg->regs,
+						 cfg->sec.tx_tbl, cfg->sec.tx_tbl_num, 2);
 	}
 
 	qcom_qmp_phy_pcie_configure_lane(rx, cfg->regs,
-				    cfg->rx_tbl, cfg->rx_tbl_num, 1);
-	if (cfg->rx_tbl_sec)
-		qcom_qmp_phy_pcie_configure_lane(rx, cfg->regs,
-					    cfg->rx_tbl_sec, cfg->rx_tbl_num_sec, 1);
+					 cfg->pri.rx_tbl, cfg->pri.rx_tbl_num, 1);
+	qcom_qmp_phy_pcie_configure_lane(rx, cfg->regs,
+					 cfg->sec.rx_tbl, cfg->sec.rx_tbl_num, 1);
 
 	if (cfg->is_dual_lane_phy) {
 		qcom_qmp_phy_pcie_configure_lane(qphy->rx2, cfg->regs,
-					    cfg->rx_tbl, cfg->rx_tbl_num, 2);
-		if (cfg->rx_tbl_sec)
-			qcom_qmp_phy_pcie_configure_lane(qphy->rx2, cfg->regs,
-						    cfg->rx_tbl_sec,
-						    cfg->rx_tbl_num_sec, 2);
+						 cfg->pri.rx_tbl, cfg->pri.rx_tbl_num, 2);
+		qcom_qmp_phy_pcie_configure_lane(qphy->rx2, cfg->regs,
+						 cfg->sec.rx_tbl, cfg->sec.rx_tbl_num, 2);
 	}
 
-	qcom_qmp_phy_pcie_configure(pcs, cfg->regs, cfg->pcs_tbl, cfg->pcs_tbl_num);
-	if (cfg->pcs_tbl_sec)
-		qcom_qmp_phy_pcie_configure(pcs, cfg->regs, cfg->pcs_tbl_sec,
-				       cfg->pcs_tbl_num_sec);
+	qcom_qmp_phy_pcie_configure(pcs, cfg->regs, cfg->pri.pcs_tbl, cfg->pri.pcs_tbl_num);
+	qcom_qmp_phy_pcie_configure(pcs, cfg->regs, cfg->sec.pcs_tbl, cfg->sec.pcs_tbl_num);
 
-	qcom_qmp_phy_pcie_configure(pcs_misc, cfg->regs, cfg->pcs_misc_tbl,
-			       cfg->pcs_misc_tbl_num);
-	if (cfg->pcs_misc_tbl_sec)
-		qcom_qmp_phy_pcie_configure(pcs_misc, cfg->regs, cfg->pcs_misc_tbl_sec,
-				       cfg->pcs_misc_tbl_num_sec);
+	qcom_qmp_phy_pcie_configure(pcs_misc, cfg->regs, cfg->pri.pcs_misc_tbl,
+			       cfg->pri.pcs_misc_tbl_num);
+	qcom_qmp_phy_pcie_configure(pcs_misc, cfg->regs, cfg->sec.pcs_misc_tbl,
+			       cfg->sec.pcs_misc_tbl_num);
 
 	/*
 	 * Pull out PHY from POWER DOWN state.
-- 
2.35.1

[RFC PATCH 2/4] phy: qcom-qmp-pcie: suppor separate tables for EP mode

[Dmitry Baryshkov]

The PCIe QMP PHY requires different programming sequences when being
used for the RC (Root Complex) or for the EP (End Point) modes. Allow
selecting the submode and thus selecting a set of PHY programming
tables.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 33 ++++++++++++++++--------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
index 23ca5848c4a8..898288c1cd7d 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
@@ -1368,7 +1368,8 @@ struct qmp_phy_cfg {
 
 	/* Init sequence for PHY blocks - serdes, tx, rx, pcs */
 	struct qmp_phy_cfg_tables pri;
-	struct qmp_phy_cfg_tables sec;
+	struct qmp_phy_cfg_tables sec_rc; /* for the RC only */
+	struct qmp_phy_cfg_tables sec_ep; /* for the EP only */
 
 	/* clock ids to be requested */
 	const char * const *clk_list;
@@ -1418,6 +1419,7 @@ struct qmp_phy_cfg {
  * @index: lane index
  * @qmp: QMP phy to which this lane belongs
  * @mode: current PHY mode
+ * @sec: currently selected PHY init table set
  */
 struct qmp_phy {
 	struct phy *phy;
@@ -1433,6 +1435,7 @@ struct qmp_phy {
 	unsigned int index;
 	struct qcom_qmp *qmp;
 	enum phy_mode mode;
+	const struct qmp_phy_cfg_tables *sec;
 };
 
 /**
@@ -1683,7 +1686,7 @@ static const struct qmp_phy_cfg sm8250_qmp_gen3x1_pciephy_cfg = {
 	.pcs_misc_tbl		= sm8250_qmp_pcie_pcs_misc_tbl,
 	.pcs_misc_tbl_num	= ARRAY_SIZE(sm8250_qmp_pcie_pcs_misc_tbl),
 	},
-	.sec = {
+	.sec_rc = {
 	.serdes_tbl		= sm8250_qmp_gen3x1_pcie_serdes_tbl,
 	.serdes_tbl_num		= ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_serdes_tbl),
 	.rx_tbl			= sm8250_qmp_gen3x1_pcie_rx_tbl,
@@ -1726,7 +1729,7 @@ static const struct qmp_phy_cfg sm8250_qmp_gen3x2_pciephy_cfg = {
 	.pcs_misc_tbl		= sm8250_qmp_pcie_pcs_misc_tbl,
 	.pcs_misc_tbl_num	= ARRAY_SIZE(sm8250_qmp_pcie_pcs_misc_tbl),
 	},
-	.sec = {
+	.sec_rc = {
 	.tx_tbl			= sm8250_qmp_gen3x2_pcie_tx_tbl,
 	.tx_tbl_num		= ARRAY_SIZE(sm8250_qmp_gen3x2_pcie_tx_tbl),
 	.rx_tbl			= sm8250_qmp_gen3x2_pcie_rx_tbl,
@@ -1951,7 +1954,7 @@ static int qcom_qmp_phy_pcie_serdes_init(struct qmp_phy *qphy)
 	void __iomem *serdes = qphy->serdes;
 
 	qcom_qmp_phy_pcie_configure(serdes, cfg->regs, cfg->pri.serdes_tbl, cfg->pri.serdes_tbl_num);
-	qcom_qmp_phy_pcie_configure(serdes, cfg->regs, cfg->sec.serdes_tbl, cfg->sec.serdes_tbl_num);
+	qcom_qmp_phy_pcie_configure(serdes, cfg->regs, qphy->sec->serdes_tbl, qphy->sec->serdes_tbl_num);
 
 	return 0;
 }
@@ -2045,6 +2048,9 @@ static int qcom_qmp_phy_pcie_power_on(struct phy *phy)
 	unsigned int mask, val, ready;
 	int ret;
 
+	if (!qphy->sec)
+		qphy->sec = &cfg->sec_rc;
+
 	qcom_qmp_phy_pcie_serdes_init(qphy);
 
 	ret = clk_prepare_enable(qphy->pipe_clk);
@@ -2057,35 +2063,35 @@ static int qcom_qmp_phy_pcie_power_on(struct phy *phy)
 	qcom_qmp_phy_pcie_configure_lane(tx, cfg->regs,
 					 cfg->pri.tx_tbl, cfg->pri.tx_tbl_num, 1);
 	qcom_qmp_phy_pcie_configure_lane(tx, cfg->regs,
-					 cfg->sec.tx_tbl, cfg->sec.tx_tbl_num, 1);
+					 qphy->sec->tx_tbl, qphy->sec->tx_tbl_num, 1);
 
 	/* Configuration for other LANE for USB-DP combo PHY */
 	if (cfg->is_dual_lane_phy) {
 		qcom_qmp_phy_pcie_configure_lane(qphy->tx2, cfg->regs,
 						 cfg->pri.tx_tbl, cfg->pri.tx_tbl_num, 2);
 		qcom_qmp_phy_pcie_configure_lane(qphy->tx2, cfg->regs,
-						 cfg->sec.tx_tbl, cfg->sec.tx_tbl_num, 2);
+						 qphy->sec->tx_tbl, qphy->sec->tx_tbl_num, 2);
 	}
 
 	qcom_qmp_phy_pcie_configure_lane(rx, cfg->regs,
 					 cfg->pri.rx_tbl, cfg->pri.rx_tbl_num, 1);
 	qcom_qmp_phy_pcie_configure_lane(rx, cfg->regs,
-					 cfg->sec.rx_tbl, cfg->sec.rx_tbl_num, 1);
+					 qphy->sec->rx_tbl, qphy->sec->rx_tbl_num, 1);
 
 	if (cfg->is_dual_lane_phy) {
 		qcom_qmp_phy_pcie_configure_lane(qphy->rx2, cfg->regs,
 						 cfg->pri.rx_tbl, cfg->pri.rx_tbl_num, 2);
 		qcom_qmp_phy_pcie_configure_lane(qphy->rx2, cfg->regs,
-						 cfg->sec.rx_tbl, cfg->sec.rx_tbl_num, 2);
+						 qphy->sec->rx_tbl, qphy->sec->rx_tbl_num, 2);
 	}
 
 	qcom_qmp_phy_pcie_configure(pcs, cfg->regs, cfg->pri.pcs_tbl, cfg->pri.pcs_tbl_num);
-	qcom_qmp_phy_pcie_configure(pcs, cfg->regs, cfg->sec.pcs_tbl, cfg->sec.pcs_tbl_num);
+	qcom_qmp_phy_pcie_configure(pcs, cfg->regs, qphy->sec->pcs_tbl, qphy->sec->pcs_tbl_num);
 
 	qcom_qmp_phy_pcie_configure(pcs_misc, cfg->regs, cfg->pri.pcs_misc_tbl,
 			       cfg->pri.pcs_misc_tbl_num);
-	qcom_qmp_phy_pcie_configure(pcs_misc, cfg->regs, cfg->sec.pcs_misc_tbl,
-			       cfg->sec.pcs_misc_tbl_num);
+	qcom_qmp_phy_pcie_configure(pcs_misc, cfg->regs, qphy->sec->pcs_misc_tbl,
+			       qphy->sec->pcs_misc_tbl_num);
 
 	/*
 	 * Pull out PHY from POWER DOWN state.
@@ -2187,6 +2193,11 @@ static int qcom_qmp_phy_pcie_set_mode(struct phy *phy,
 
 	qphy->mode = mode;
 
+	if (submode)
+		qphy->sec = &qphy->cfg->sec_ep;
+	else
+		qphy->sec = &qphy->cfg->sec_rc;
+
 	return 0;
 }
 
-- 
2.35.1

[RFC PATCH 3/4] PCI: qcom: call phy_set_mode_ext()

[Dmitry Baryshkov]

Call phy_set_mode_ext() to notify the PHY driver that the PHY is being
used in the RC mode.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 5ed164c2afa3..50bf3fefef7a 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1486,6 +1486,10 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
 	if (ret)
 		return ret;
 
+	ret = phy_set_mode_ext(pcie->phy, PHY_MODE_PCIE, 0);
+	if (ret)
+		return ret;
+
 	ret = phy_power_on(pcie->phy);
 	if (ret)
 		goto err_deinit;
-- 
2.35.1

[RFC PATCH 4/4] PCI: qcom-ep: call phy_set_mode_ext()

[Dmitry Baryshkov]

Call phy_set_mode_ext() to notify the PHY driver that the PHY is being
used in the EP mode.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/pci/controller/dwc/pcie-qcom-ep.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index ec99116ad05c..11f887a2f7c4 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -240,6 +240,10 @@ static int qcom_pcie_enable_resources(struct qcom_pcie_ep *pcie_ep)
 	if (ret)
 		goto err_disable_clk;
 
+	ret = phy_set_mode_ext(pcie->phy, PHY_MODE_PCIE, 1);
+	if (ret)
+		goto err_phy_exit;
+
 	ret = phy_power_on(pcie_ep->phy);
 	if (ret)
 		goto err_phy_exit;
-- 
2.35.1

Re: [RFC PATCH 1/4] phy: qcom-qmp-pcie: split register tables into primary and secondary part

[Manivannan Sadhasivam]

On Tue, Jul 19, 2022 at 11:06:23PM +0300, Dmitry Baryshkov wrote:
> Split register tables list into primary and secondary parts. While we
> are at it, drop unused if (table) conditions, since the function
> qcom_qmp_phy_pcie_configure_lane() has this check anyway.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 144 ++++++++++++-----------
>  1 file changed, 77 insertions(+), 67 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> index 2d65e1f56bfc..23ca5848c4a8 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> @@ -1346,34 +1346,29 @@ static const struct qmp_phy_init_tbl sm8450_qmp_gen4x2_pcie_pcs_misc_tbl[] = {
>  
>  struct qmp_phy;
>  
> -/* struct qmp_phy_cfg - per-PHY initialization config */
> -struct qmp_phy_cfg {
> -	/* phy-type - PCIE/UFS/USB */
> -	unsigned int type;
> -	/* number of lanes provided by phy */
> -	int nlanes;
> -
> -	/* Init sequence for PHY blocks - serdes, tx, rx, pcs */
> +struct qmp_phy_cfg_tables {
>  	const struct qmp_phy_init_tbl *serdes_tbl;
>  	int serdes_tbl_num;
> -	const struct qmp_phy_init_tbl *serdes_tbl_sec;
> -	int serdes_tbl_num_sec;
>  	const struct qmp_phy_init_tbl *tx_tbl;
>  	int tx_tbl_num;
> -	const struct qmp_phy_init_tbl *tx_tbl_sec;
> -	int tx_tbl_num_sec;
>  	const struct qmp_phy_init_tbl *rx_tbl;
>  	int rx_tbl_num;
> -	const struct qmp_phy_init_tbl *rx_tbl_sec;
> -	int rx_tbl_num_sec;
>  	const struct qmp_phy_init_tbl *pcs_tbl;
>  	int pcs_tbl_num;
> -	const struct qmp_phy_init_tbl *pcs_tbl_sec;
> -	int pcs_tbl_num_sec;
>  	const struct qmp_phy_init_tbl *pcs_misc_tbl;
>  	int pcs_misc_tbl_num;
> -	const struct qmp_phy_init_tbl *pcs_misc_tbl_sec;
> -	int pcs_misc_tbl_num_sec;
> +};
> +
> +/* struct qmp_phy_cfg - per-PHY initialization config */
> +struct qmp_phy_cfg {
> +	/* phy-type - PCIE/UFS/USB */
> +	unsigned int type;
> +	/* number of lanes provided by phy */
> +	int nlanes;
> +
> +	/* Init sequence for PHY blocks - serdes, tx, rx, pcs */
> +	struct qmp_phy_cfg_tables pri;

s/pri/primary/g

> +	struct qmp_phy_cfg_tables sec;

s/sec/secondary/g

I think it'd be good to add a comment on what the secondary table represents.
This will help folks trying to port the downstream drivers.

Rest LGTM!

Thanks,
Mani

>  
>  	/* clock ids to be requested */
>  	const char * const *clk_list;
> @@ -1517,6 +1512,7 @@ static const struct qmp_phy_cfg ipq8074_pciephy_cfg = {
>  	.type			= PHY_TYPE_PCIE,
>  	.nlanes			= 1,
>  
> +	.pri = {
>  	.serdes_tbl		= ipq8074_pcie_serdes_tbl,
>  	.serdes_tbl_num		= ARRAY_SIZE(ipq8074_pcie_serdes_tbl),
>  	.tx_tbl			= ipq8074_pcie_tx_tbl,
> @@ -1525,6 +1521,7 @@ static const struct qmp_phy_cfg ipq8074_pciephy_cfg = {
>  	.rx_tbl_num		= ARRAY_SIZE(ipq8074_pcie_rx_tbl),
>  	.pcs_tbl		= ipq8074_pcie_pcs_tbl,
>  	.pcs_tbl_num		= ARRAY_SIZE(ipq8074_pcie_pcs_tbl),
> +	},
>  	.clk_list		= ipq8074_pciephy_clk_l,
>  	.num_clks		= ARRAY_SIZE(ipq8074_pciephy_clk_l),
>  	.reset_list		= ipq8074_pciephy_reset_l,
> @@ -1546,6 +1543,7 @@ static const struct qmp_phy_cfg ipq8074_pciephy_gen3_cfg = {
>  	.type			= PHY_TYPE_PCIE,
>  	.nlanes			= 1,
>  
> +	.pri = {
>  	.serdes_tbl		= ipq8074_pcie_gen3_serdes_tbl,
>  	.serdes_tbl_num		= ARRAY_SIZE(ipq8074_pcie_gen3_serdes_tbl),
>  	.tx_tbl			= ipq8074_pcie_gen3_tx_tbl,
> @@ -1554,6 +1552,7 @@ static const struct qmp_phy_cfg ipq8074_pciephy_gen3_cfg = {
>  	.rx_tbl_num		= ARRAY_SIZE(ipq8074_pcie_gen3_rx_tbl),
>  	.pcs_tbl		= ipq8074_pcie_gen3_pcs_tbl,
>  	.pcs_tbl_num		= ARRAY_SIZE(ipq8074_pcie_gen3_pcs_tbl),
> +	},
>  	.clk_list		= ipq8074_pciephy_clk_l,
>  	.num_clks		= ARRAY_SIZE(ipq8074_pciephy_clk_l),
>  	.reset_list		= ipq8074_pciephy_reset_l,
> @@ -1576,6 +1575,7 @@ static const struct qmp_phy_cfg ipq6018_pciephy_cfg = {
>  	.type			= PHY_TYPE_PCIE,
>  	.nlanes			= 1,
>  
> +	.pri = {
>  	.serdes_tbl		= ipq6018_pcie_serdes_tbl,
>  	.serdes_tbl_num		= ARRAY_SIZE(ipq6018_pcie_serdes_tbl),
>  	.tx_tbl			= ipq6018_pcie_tx_tbl,
> @@ -1586,6 +1586,7 @@ static const struct qmp_phy_cfg ipq6018_pciephy_cfg = {
>  	.pcs_tbl_num		= ARRAY_SIZE(ipq6018_pcie_pcs_tbl),
>  	.pcs_misc_tbl		= ipq6018_pcie_pcs_misc_tbl,
>  	.pcs_misc_tbl_num	= ARRAY_SIZE(ipq6018_pcie_pcs_misc_tbl),
> +	},
>  	.clk_list		= ipq8074_pciephy_clk_l,
>  	.num_clks		= ARRAY_SIZE(ipq8074_pciephy_clk_l),
>  	.reset_list		= ipq8074_pciephy_reset_l,
> @@ -1606,6 +1607,7 @@ static const struct qmp_phy_cfg sdm845_qmp_pciephy_cfg = {
>  	.type = PHY_TYPE_PCIE,
>  	.nlanes = 1,
>  
> +	.pri = {
>  	.serdes_tbl		= sdm845_qmp_pcie_serdes_tbl,
>  	.serdes_tbl_num		= ARRAY_SIZE(sdm845_qmp_pcie_serdes_tbl),
>  	.tx_tbl			= sdm845_qmp_pcie_tx_tbl,
> @@ -1616,6 +1618,7 @@ static const struct qmp_phy_cfg sdm845_qmp_pciephy_cfg = {
>  	.pcs_tbl_num		= ARRAY_SIZE(sdm845_qmp_pcie_pcs_tbl),
>  	.pcs_misc_tbl		= sdm845_qmp_pcie_pcs_misc_tbl,
>  	.pcs_misc_tbl_num	= ARRAY_SIZE(sdm845_qmp_pcie_pcs_misc_tbl),
> +	},
>  	.clk_list		= sdm845_pciephy_clk_l,
>  	.num_clks		= ARRAY_SIZE(sdm845_pciephy_clk_l),
>  	.reset_list		= sdm845_pciephy_reset_l,
> @@ -1637,6 +1640,7 @@ static const struct qmp_phy_cfg sdm845_qhp_pciephy_cfg = {
>  	.type = PHY_TYPE_PCIE,
>  	.nlanes = 1,
>  
> +	.pri = {
>  	.serdes_tbl		= sdm845_qhp_pcie_serdes_tbl,
>  	.serdes_tbl_num		= ARRAY_SIZE(sdm845_qhp_pcie_serdes_tbl),
>  	.tx_tbl			= sdm845_qhp_pcie_tx_tbl,
> @@ -1645,6 +1649,7 @@ static const struct qmp_phy_cfg sdm845_qhp_pciephy_cfg = {
>  	.rx_tbl_num		= ARRAY_SIZE(sdm845_qhp_pcie_rx_tbl),
>  	.pcs_tbl		= sdm845_qhp_pcie_pcs_tbl,
>  	.pcs_tbl_num		= ARRAY_SIZE(sdm845_qhp_pcie_pcs_tbl),
> +	},
>  	.clk_list		= sdm845_pciephy_clk_l,
>  	.num_clks		= ARRAY_SIZE(sdm845_pciephy_clk_l),
>  	.reset_list		= sdm845_pciephy_reset_l,
> @@ -1666,24 +1671,28 @@ static const struct qmp_phy_cfg sm8250_qmp_gen3x1_pciephy_cfg = {
>  	.type = PHY_TYPE_PCIE,
>  	.nlanes = 1,
>  
> +	.pri = {
>  	.serdes_tbl		= sm8250_qmp_pcie_serdes_tbl,
>  	.serdes_tbl_num		= ARRAY_SIZE(sm8250_qmp_pcie_serdes_tbl),
> -	.serdes_tbl_sec		= sm8250_qmp_gen3x1_pcie_serdes_tbl,
> -	.serdes_tbl_num_sec	= ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_serdes_tbl),
>  	.tx_tbl			= sm8250_qmp_pcie_tx_tbl,
>  	.tx_tbl_num		= ARRAY_SIZE(sm8250_qmp_pcie_tx_tbl),
>  	.rx_tbl			= sm8250_qmp_pcie_rx_tbl,
>  	.rx_tbl_num		= ARRAY_SIZE(sm8250_qmp_pcie_rx_tbl),
> -	.rx_tbl_sec		= sm8250_qmp_gen3x1_pcie_rx_tbl,
> -	.rx_tbl_num_sec		= ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_rx_tbl),
>  	.pcs_tbl		= sm8250_qmp_pcie_pcs_tbl,
>  	.pcs_tbl_num		= ARRAY_SIZE(sm8250_qmp_pcie_pcs_tbl),
> -	.pcs_tbl_sec		= sm8250_qmp_gen3x1_pcie_pcs_tbl,
> -	.pcs_tbl_num_sec		= ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_pcs_tbl),
>  	.pcs_misc_tbl		= sm8250_qmp_pcie_pcs_misc_tbl,
>  	.pcs_misc_tbl_num	= ARRAY_SIZE(sm8250_qmp_pcie_pcs_misc_tbl),
> -	.pcs_misc_tbl_sec		= sm8250_qmp_gen3x1_pcie_pcs_misc_tbl,
> -	.pcs_misc_tbl_num_sec	= ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_pcs_misc_tbl),
> +	},
> +	.sec = {
> +	.serdes_tbl		= sm8250_qmp_gen3x1_pcie_serdes_tbl,
> +	.serdes_tbl_num		= ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_serdes_tbl),
> +	.rx_tbl			= sm8250_qmp_gen3x1_pcie_rx_tbl,
> +	.rx_tbl_num		= ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_rx_tbl),
> +	.pcs_tbl		= sm8250_qmp_gen3x1_pcie_pcs_tbl,
> +	.pcs_tbl_num		= ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_pcs_tbl),
> +	.pcs_misc_tbl		= sm8250_qmp_gen3x1_pcie_pcs_misc_tbl,
> +	.pcs_misc_tbl_num	= ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_pcs_misc_tbl),
> +	},
>  	.clk_list		= sdm845_pciephy_clk_l,
>  	.num_clks		= ARRAY_SIZE(sdm845_pciephy_clk_l),
>  	.reset_list		= sdm845_pciephy_reset_l,
> @@ -1705,24 +1714,28 @@ static const struct qmp_phy_cfg sm8250_qmp_gen3x2_pciephy_cfg = {
>  	.type = PHY_TYPE_PCIE,
>  	.nlanes = 2,
>  
> +	.pri = {
>  	.serdes_tbl		= sm8250_qmp_pcie_serdes_tbl,
>  	.serdes_tbl_num		= ARRAY_SIZE(sm8250_qmp_pcie_serdes_tbl),
>  	.tx_tbl			= sm8250_qmp_pcie_tx_tbl,
>  	.tx_tbl_num		= ARRAY_SIZE(sm8250_qmp_pcie_tx_tbl),
> -	.tx_tbl_sec		= sm8250_qmp_gen3x2_pcie_tx_tbl,
> -	.tx_tbl_num_sec		= ARRAY_SIZE(sm8250_qmp_gen3x2_pcie_tx_tbl),
>  	.rx_tbl			= sm8250_qmp_pcie_rx_tbl,
>  	.rx_tbl_num		= ARRAY_SIZE(sm8250_qmp_pcie_rx_tbl),
> -	.rx_tbl_sec		= sm8250_qmp_gen3x2_pcie_rx_tbl,
> -	.rx_tbl_num_sec		= ARRAY_SIZE(sm8250_qmp_gen3x2_pcie_rx_tbl),
>  	.pcs_tbl		= sm8250_qmp_pcie_pcs_tbl,
>  	.pcs_tbl_num		= ARRAY_SIZE(sm8250_qmp_pcie_pcs_tbl),
> -	.pcs_tbl_sec		= sm8250_qmp_gen3x2_pcie_pcs_tbl,
> -	.pcs_tbl_num_sec		= ARRAY_SIZE(sm8250_qmp_gen3x2_pcie_pcs_tbl),
>  	.pcs_misc_tbl		= sm8250_qmp_pcie_pcs_misc_tbl,
>  	.pcs_misc_tbl_num	= ARRAY_SIZE(sm8250_qmp_pcie_pcs_misc_tbl),
> -	.pcs_misc_tbl_sec		= sm8250_qmp_gen3x2_pcie_pcs_misc_tbl,
> -	.pcs_misc_tbl_num_sec	= ARRAY_SIZE(sm8250_qmp_gen3x2_pcie_pcs_misc_tbl),
> +	},
> +	.sec = {
> +	.tx_tbl			= sm8250_qmp_gen3x2_pcie_tx_tbl,
> +	.tx_tbl_num		= ARRAY_SIZE(sm8250_qmp_gen3x2_pcie_tx_tbl),
> +	.rx_tbl			= sm8250_qmp_gen3x2_pcie_rx_tbl,
> +	.rx_tbl_num		= ARRAY_SIZE(sm8250_qmp_gen3x2_pcie_rx_tbl),
> +	.pcs_tbl		= sm8250_qmp_gen3x2_pcie_pcs_tbl,
> +	.pcs_tbl_num		= ARRAY_SIZE(sm8250_qmp_gen3x2_pcie_pcs_tbl),
> +	.pcs_misc_tbl		= sm8250_qmp_gen3x2_pcie_pcs_misc_tbl,
> +	.pcs_misc_tbl_num	= ARRAY_SIZE(sm8250_qmp_gen3x2_pcie_pcs_misc_tbl),
> +	},
>  	.clk_list		= sdm845_pciephy_clk_l,
>  	.num_clks		= ARRAY_SIZE(sdm845_pciephy_clk_l),
>  	.reset_list		= sdm845_pciephy_reset_l,
> @@ -1745,6 +1758,7 @@ static const struct qmp_phy_cfg msm8998_pciephy_cfg = {
>  	.type			= PHY_TYPE_PCIE,
>  	.nlanes			= 1,
>  
> +	.pri = {
>  	.serdes_tbl		= msm8998_pcie_serdes_tbl,
>  	.serdes_tbl_num		= ARRAY_SIZE(msm8998_pcie_serdes_tbl),
>  	.tx_tbl			= msm8998_pcie_tx_tbl,
> @@ -1753,6 +1767,7 @@ static const struct qmp_phy_cfg msm8998_pciephy_cfg = {
>  	.rx_tbl_num		= ARRAY_SIZE(msm8998_pcie_rx_tbl),
>  	.pcs_tbl		= msm8998_pcie_pcs_tbl,
>  	.pcs_tbl_num		= ARRAY_SIZE(msm8998_pcie_pcs_tbl),
> +	},
>  	.clk_list		= msm8996_phy_clk_l,
>  	.num_clks		= ARRAY_SIZE(msm8996_phy_clk_l),
>  	.reset_list		= ipq8074_pciephy_reset_l,
> @@ -1770,6 +1785,7 @@ static const struct qmp_phy_cfg sc8180x_pciephy_cfg = {
>  	.type = PHY_TYPE_PCIE,
>  	.nlanes = 1,
>  
> +	.pri = {
>  	.serdes_tbl		= sc8180x_qmp_pcie_serdes_tbl,
>  	.serdes_tbl_num		= ARRAY_SIZE(sc8180x_qmp_pcie_serdes_tbl),
>  	.tx_tbl			= sc8180x_qmp_pcie_tx_tbl,
> @@ -1780,6 +1796,7 @@ static const struct qmp_phy_cfg sc8180x_pciephy_cfg = {
>  	.pcs_tbl_num		= ARRAY_SIZE(sc8180x_qmp_pcie_pcs_tbl),
>  	.pcs_misc_tbl		= sc8180x_qmp_pcie_pcs_misc_tbl,
>  	.pcs_misc_tbl_num	= ARRAY_SIZE(sc8180x_qmp_pcie_pcs_misc_tbl),
> +	},
>  	.clk_list		= sdm845_pciephy_clk_l,
>  	.num_clks		= ARRAY_SIZE(sdm845_pciephy_clk_l),
>  	.reset_list		= sdm845_pciephy_reset_l,
> @@ -1800,6 +1817,7 @@ static const struct qmp_phy_cfg sdx55_qmp_pciephy_cfg = {
>  	.type = PHY_TYPE_PCIE,
>  	.nlanes = 2,
>  
> +	.pri = {
>  	.serdes_tbl		= sdx55_qmp_pcie_serdes_tbl,
>  	.serdes_tbl_num		= ARRAY_SIZE(sdx55_qmp_pcie_serdes_tbl),
>  	.tx_tbl			= sdx55_qmp_pcie_tx_tbl,
> @@ -1810,6 +1828,7 @@ static const struct qmp_phy_cfg sdx55_qmp_pciephy_cfg = {
>  	.pcs_tbl_num		= ARRAY_SIZE(sdx55_qmp_pcie_pcs_tbl),
>  	.pcs_misc_tbl		= sdx55_qmp_pcie_pcs_misc_tbl,
>  	.pcs_misc_tbl_num	= ARRAY_SIZE(sdx55_qmp_pcie_pcs_misc_tbl),
> +	},
>  	.clk_list		= sdm845_pciephy_clk_l,
>  	.num_clks		= ARRAY_SIZE(sdm845_pciephy_clk_l),
>  	.reset_list		= sdm845_pciephy_reset_l,
> @@ -1832,6 +1851,7 @@ static const struct qmp_phy_cfg sm8450_qmp_gen3x1_pciephy_cfg = {
>  	.type = PHY_TYPE_PCIE,
>  	.nlanes = 1,
>  
> +	.pri = {
>  	.serdes_tbl		= sm8450_qmp_gen3x1_pcie_serdes_tbl,
>  	.serdes_tbl_num		= ARRAY_SIZE(sm8450_qmp_gen3x1_pcie_serdes_tbl),
>  	.tx_tbl			= sm8450_qmp_gen3x1_pcie_tx_tbl,
> @@ -1842,6 +1862,7 @@ static const struct qmp_phy_cfg sm8450_qmp_gen3x1_pciephy_cfg = {
>  	.pcs_tbl_num		= ARRAY_SIZE(sm8450_qmp_gen3x1_pcie_pcs_tbl),
>  	.pcs_misc_tbl		= sm8450_qmp_gen3x1_pcie_pcs_misc_tbl,
>  	.pcs_misc_tbl_num	= ARRAY_SIZE(sm8450_qmp_gen3x1_pcie_pcs_misc_tbl),
> +	},
>  	.clk_list		= sdm845_pciephy_clk_l,
>  	.num_clks		= ARRAY_SIZE(sdm845_pciephy_clk_l),
>  	.reset_list		= sdm845_pciephy_reset_l,
> @@ -1863,6 +1884,7 @@ static const struct qmp_phy_cfg sm8450_qmp_gen4x2_pciephy_cfg = {
>  	.type = PHY_TYPE_PCIE,
>  	.nlanes = 2,
>  
> +	.pri = {
>  	.serdes_tbl		= sm8450_qmp_gen4x2_pcie_serdes_tbl,
>  	.serdes_tbl_num		= ARRAY_SIZE(sm8450_qmp_gen4x2_pcie_serdes_tbl),
>  	.tx_tbl			= sm8450_qmp_gen4x2_pcie_tx_tbl,
> @@ -1873,6 +1895,7 @@ static const struct qmp_phy_cfg sm8450_qmp_gen4x2_pciephy_cfg = {
>  	.pcs_tbl_num		= ARRAY_SIZE(sm8450_qmp_gen4x2_pcie_pcs_tbl),
>  	.pcs_misc_tbl		= sm8450_qmp_gen4x2_pcie_pcs_misc_tbl,
>  	.pcs_misc_tbl_num	= ARRAY_SIZE(sm8450_qmp_gen4x2_pcie_pcs_misc_tbl),
> +	},
>  	.clk_list		= sdm845_pciephy_clk_l,
>  	.num_clks		= ARRAY_SIZE(sdm845_pciephy_clk_l),
>  	.reset_list		= sdm845_pciephy_reset_l,
> @@ -1926,13 +1949,9 @@ static int qcom_qmp_phy_pcie_serdes_init(struct qmp_phy *qphy)
>  {
>  	const struct qmp_phy_cfg *cfg = qphy->cfg;
>  	void __iomem *serdes = qphy->serdes;
> -	const struct qmp_phy_init_tbl *serdes_tbl = cfg->serdes_tbl;
> -	int serdes_tbl_num = cfg->serdes_tbl_num;
>  
> -	qcom_qmp_phy_pcie_configure(serdes, cfg->regs, serdes_tbl, serdes_tbl_num);
> -	if (cfg->serdes_tbl_sec)
> -		qcom_qmp_phy_pcie_configure(serdes, cfg->regs, cfg->serdes_tbl_sec,
> -				       cfg->serdes_tbl_num_sec);
> +	qcom_qmp_phy_pcie_configure(serdes, cfg->regs, cfg->pri.serdes_tbl, cfg->pri.serdes_tbl_num);
> +	qcom_qmp_phy_pcie_configure(serdes, cfg->regs, cfg->sec.serdes_tbl, cfg->sec.serdes_tbl_num);
>  
>  	return 0;
>  }
> @@ -2036,46 +2055,37 @@ static int qcom_qmp_phy_pcie_power_on(struct phy *phy)
>  
>  	/* Tx, Rx, and PCS configurations */
>  	qcom_qmp_phy_pcie_configure_lane(tx, cfg->regs,
> -				    cfg->tx_tbl, cfg->tx_tbl_num, 1);
> -	if (cfg->tx_tbl_sec)
> -		qcom_qmp_phy_pcie_configure_lane(tx, cfg->regs, cfg->tx_tbl_sec,
> -					    cfg->tx_tbl_num_sec, 1);
> +					 cfg->pri.tx_tbl, cfg->pri.tx_tbl_num, 1);
> +	qcom_qmp_phy_pcie_configure_lane(tx, cfg->regs,
> +					 cfg->sec.tx_tbl, cfg->sec.tx_tbl_num, 1);
>  
>  	/* Configuration for other LANE for USB-DP combo PHY */
>  	if (cfg->is_dual_lane_phy) {
>  		qcom_qmp_phy_pcie_configure_lane(qphy->tx2, cfg->regs,
> -					    cfg->tx_tbl, cfg->tx_tbl_num, 2);
> -		if (cfg->tx_tbl_sec)
> -			qcom_qmp_phy_pcie_configure_lane(qphy->tx2, cfg->regs,
> -						    cfg->tx_tbl_sec,
> -						    cfg->tx_tbl_num_sec, 2);
> +						 cfg->pri.tx_tbl, cfg->pri.tx_tbl_num, 2);
> +		qcom_qmp_phy_pcie_configure_lane(qphy->tx2, cfg->regs,
> +						 cfg->sec.tx_tbl, cfg->sec.tx_tbl_num, 2);
>  	}
>  
>  	qcom_qmp_phy_pcie_configure_lane(rx, cfg->regs,
> -				    cfg->rx_tbl, cfg->rx_tbl_num, 1);
> -	if (cfg->rx_tbl_sec)
> -		qcom_qmp_phy_pcie_configure_lane(rx, cfg->regs,
> -					    cfg->rx_tbl_sec, cfg->rx_tbl_num_sec, 1);
> +					 cfg->pri.rx_tbl, cfg->pri.rx_tbl_num, 1);
> +	qcom_qmp_phy_pcie_configure_lane(rx, cfg->regs,
> +					 cfg->sec.rx_tbl, cfg->sec.rx_tbl_num, 1);
>  
>  	if (cfg->is_dual_lane_phy) {
>  		qcom_qmp_phy_pcie_configure_lane(qphy->rx2, cfg->regs,
> -					    cfg->rx_tbl, cfg->rx_tbl_num, 2);
> -		if (cfg->rx_tbl_sec)
> -			qcom_qmp_phy_pcie_configure_lane(qphy->rx2, cfg->regs,
> -						    cfg->rx_tbl_sec,
> -						    cfg->rx_tbl_num_sec, 2);
> +						 cfg->pri.rx_tbl, cfg->pri.rx_tbl_num, 2);
> +		qcom_qmp_phy_pcie_configure_lane(qphy->rx2, cfg->regs,
> +						 cfg->sec.rx_tbl, cfg->sec.rx_tbl_num, 2);
>  	}
>  
> -	qcom_qmp_phy_pcie_configure(pcs, cfg->regs, cfg->pcs_tbl, cfg->pcs_tbl_num);
> -	if (cfg->pcs_tbl_sec)
> -		qcom_qmp_phy_pcie_configure(pcs, cfg->regs, cfg->pcs_tbl_sec,
> -				       cfg->pcs_tbl_num_sec);
> +	qcom_qmp_phy_pcie_configure(pcs, cfg->regs, cfg->pri.pcs_tbl, cfg->pri.pcs_tbl_num);
> +	qcom_qmp_phy_pcie_configure(pcs, cfg->regs, cfg->sec.pcs_tbl, cfg->sec.pcs_tbl_num);
>  
> -	qcom_qmp_phy_pcie_configure(pcs_misc, cfg->regs, cfg->pcs_misc_tbl,
> -			       cfg->pcs_misc_tbl_num);
> -	if (cfg->pcs_misc_tbl_sec)
> -		qcom_qmp_phy_pcie_configure(pcs_misc, cfg->regs, cfg->pcs_misc_tbl_sec,
> -				       cfg->pcs_misc_tbl_num_sec);
> +	qcom_qmp_phy_pcie_configure(pcs_misc, cfg->regs, cfg->pri.pcs_misc_tbl,
> +			       cfg->pri.pcs_misc_tbl_num);
> +	qcom_qmp_phy_pcie_configure(pcs_misc, cfg->regs, cfg->sec.pcs_misc_tbl,
> +			       cfg->sec.pcs_misc_tbl_num);
>  
>  	/*
>  	 * Pull out PHY from POWER DOWN state.
> -- 
> 2.35.1
> 

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

Re: [RFC PATCH 2/4] phy: qcom-qmp-pcie: suppor separate tables for EP mode

[Manivannan Sadhasivam]

s/suppor/support

On Tue, Jul 19, 2022 at 11:06:24PM +0300, Dmitry Baryshkov wrote:
> The PCIe QMP PHY requires different programming sequences when being
> used for the RC (Root Complex) or for the EP (End Point) modes. Allow
> selecting the submode and thus selecting a set of PHY programming
> tables.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 33 ++++++++++++++++--------
>  1 file changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> index 23ca5848c4a8..898288c1cd7d 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
> @@ -1368,7 +1368,8 @@ struct qmp_phy_cfg {
>  
>  	/* Init sequence for PHY blocks - serdes, tx, rx, pcs */
>  	struct qmp_phy_cfg_tables pri;
> -	struct qmp_phy_cfg_tables sec;
> +	struct qmp_phy_cfg_tables sec_rc; /* for the RC only */
> +	struct qmp_phy_cfg_tables sec_ep; /* for the EP only */

Again, a comment is needed for these members.

>  
>  	/* clock ids to be requested */
>  	const char * const *clk_list;
> @@ -1418,6 +1419,7 @@ struct qmp_phy_cfg {
>   * @index: lane index
>   * @qmp: QMP phy to which this lane belongs
>   * @mode: current PHY mode
> + * @sec: currently selected PHY init table set
>   */
>  struct qmp_phy {
>  	struct phy *phy;
> @@ -1433,6 +1435,7 @@ struct qmp_phy {
>  	unsigned int index;
>  	struct qcom_qmp *qmp;
>  	enum phy_mode mode;
> +	const struct qmp_phy_cfg_tables *sec;

Please move the pointer to the top to avoid holes. If possible organize the
members in a pattern:

ptr
struct
enum
u64
u32
u8

>  };
>  
>  /**
> @@ -1683,7 +1686,7 @@ static const struct qmp_phy_cfg sm8250_qmp_gen3x1_pciephy_cfg = {
>  	.pcs_misc_tbl		= sm8250_qmp_pcie_pcs_misc_tbl,
>  	.pcs_misc_tbl_num	= ARRAY_SIZE(sm8250_qmp_pcie_pcs_misc_tbl),
>  	},
> -	.sec = {
> +	.sec_rc = {
>  	.serdes_tbl		= sm8250_qmp_gen3x1_pcie_serdes_tbl,
>  	.serdes_tbl_num		= ARRAY_SIZE(sm8250_qmp_gen3x1_pcie_serdes_tbl),
>  	.rx_tbl			= sm8250_qmp_gen3x1_pcie_rx_tbl,
> @@ -1726,7 +1729,7 @@ static const struct qmp_phy_cfg sm8250_qmp_gen3x2_pciephy_cfg = {
>  	.pcs_misc_tbl		= sm8250_qmp_pcie_pcs_misc_tbl,
>  	.pcs_misc_tbl_num	= ARRAY_SIZE(sm8250_qmp_pcie_pcs_misc_tbl),
>  	},
> -	.sec = {
> +	.sec_rc = {
>  	.tx_tbl			= sm8250_qmp_gen3x2_pcie_tx_tbl,
>  	.tx_tbl_num		= ARRAY_SIZE(sm8250_qmp_gen3x2_pcie_tx_tbl),
>  	.rx_tbl			= sm8250_qmp_gen3x2_pcie_rx_tbl,
> @@ -1951,7 +1954,7 @@ static int qcom_qmp_phy_pcie_serdes_init(struct qmp_phy *qphy)
>  	void __iomem *serdes = qphy->serdes;
>  
>  	qcom_qmp_phy_pcie_configure(serdes, cfg->regs, cfg->pri.serdes_tbl, cfg->pri.serdes_tbl_num);
> -	qcom_qmp_phy_pcie_configure(serdes, cfg->regs, cfg->sec.serdes_tbl, cfg->sec.serdes_tbl_num);
> +	qcom_qmp_phy_pcie_configure(serdes, cfg->regs, qphy->sec->serdes_tbl, qphy->sec->serdes_tbl_num);
>  
>  	return 0;
>  }
> @@ -2045,6 +2048,9 @@ static int qcom_qmp_phy_pcie_power_on(struct phy *phy)
>  	unsigned int mask, val, ready;
>  	int ret;
>  

Here also, a comment would add a value.

Thanks,
Mani

> +	if (!qphy->sec)
> +		qphy->sec = &cfg->sec_rc;
> +
>  	qcom_qmp_phy_pcie_serdes_init(qphy);
>  
>  	ret = clk_prepare_enable(qphy->pipe_clk);
> @@ -2057,35 +2063,35 @@ static int qcom_qmp_phy_pcie_power_on(struct phy *phy)
>  	qcom_qmp_phy_pcie_configure_lane(tx, cfg->regs,
>  					 cfg->pri.tx_tbl, cfg->pri.tx_tbl_num, 1);
>  	qcom_qmp_phy_pcie_configure_lane(tx, cfg->regs,
> -					 cfg->sec.tx_tbl, cfg->sec.tx_tbl_num, 1);
> +					 qphy->sec->tx_tbl, qphy->sec->tx_tbl_num, 1);
>  
>  	/* Configuration for other LANE for USB-DP combo PHY */
>  	if (cfg->is_dual_lane_phy) {
>  		qcom_qmp_phy_pcie_configure_lane(qphy->tx2, cfg->regs,
>  						 cfg->pri.tx_tbl, cfg->pri.tx_tbl_num, 2);
>  		qcom_qmp_phy_pcie_configure_lane(qphy->tx2, cfg->regs,
> -						 cfg->sec.tx_tbl, cfg->sec.tx_tbl_num, 2);
> +						 qphy->sec->tx_tbl, qphy->sec->tx_tbl_num, 2);
>  	}
>  
>  	qcom_qmp_phy_pcie_configure_lane(rx, cfg->regs,
>  					 cfg->pri.rx_tbl, cfg->pri.rx_tbl_num, 1);
>  	qcom_qmp_phy_pcie_configure_lane(rx, cfg->regs,
> -					 cfg->sec.rx_tbl, cfg->sec.rx_tbl_num, 1);
> +					 qphy->sec->rx_tbl, qphy->sec->rx_tbl_num, 1);
>  
>  	if (cfg->is_dual_lane_phy) {
>  		qcom_qmp_phy_pcie_configure_lane(qphy->rx2, cfg->regs,
>  						 cfg->pri.rx_tbl, cfg->pri.rx_tbl_num, 2);
>  		qcom_qmp_phy_pcie_configure_lane(qphy->rx2, cfg->regs,
> -						 cfg->sec.rx_tbl, cfg->sec.rx_tbl_num, 2);
> +						 qphy->sec->rx_tbl, qphy->sec->rx_tbl_num, 2);
>  	}
>  
>  	qcom_qmp_phy_pcie_configure(pcs, cfg->regs, cfg->pri.pcs_tbl, cfg->pri.pcs_tbl_num);
> -	qcom_qmp_phy_pcie_configure(pcs, cfg->regs, cfg->sec.pcs_tbl, cfg->sec.pcs_tbl_num);
> +	qcom_qmp_phy_pcie_configure(pcs, cfg->regs, qphy->sec->pcs_tbl, qphy->sec->pcs_tbl_num);
>  
>  	qcom_qmp_phy_pcie_configure(pcs_misc, cfg->regs, cfg->pri.pcs_misc_tbl,
>  			       cfg->pri.pcs_misc_tbl_num);
> -	qcom_qmp_phy_pcie_configure(pcs_misc, cfg->regs, cfg->sec.pcs_misc_tbl,
> -			       cfg->sec.pcs_misc_tbl_num);
> +	qcom_qmp_phy_pcie_configure(pcs_misc, cfg->regs, qphy->sec->pcs_misc_tbl,
> +			       qphy->sec->pcs_misc_tbl_num);
>  
>  	/*
>  	 * Pull out PHY from POWER DOWN state.
> @@ -2187,6 +2193,11 @@ static int qcom_qmp_phy_pcie_set_mode(struct phy *phy,
>  
>  	qphy->mode = mode;
>  
> +	if (submode)
> +		qphy->sec = &qphy->cfg->sec_ep;
> +	else
> +		qphy->sec = &qphy->cfg->sec_rc;
> +
>  	return 0;
>  }
>  
> -- 
> 2.35.1
> 

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

Re: [RFC PATCH 3/4] PCI: qcom: call phy_set_mode_ext()

[Manivannan Sadhasivam]

Subject could be,

"PCI: qcom: Set PHY in RC mode"

On Tue, Jul 19, 2022 at 11:06:25PM +0300, Dmitry Baryshkov wrote:
> Call phy_set_mode_ext() to notify the PHY driver that the PHY is being
> used in the RC mode.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 5ed164c2afa3..50bf3fefef7a 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1486,6 +1486,10 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
>  	if (ret)
>  		return ret;
>  
> +	ret = phy_set_mode_ext(pcie->phy, PHY_MODE_PCIE, 0);
> +	if (ret)
> +		return ret;

err_deinit?

Thanks,
Mani

> +
>  	ret = phy_power_on(pcie->phy);
>  	if (ret)
>  		goto err_deinit;
> -- 
> 2.35.1
> 

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

Re: [RFC PATCH 4/4] PCI: qcom-ep: call phy_set_mode_ext()

[Manivannan Sadhasivam]

Subject could be,

"PCI: qcom: Set PHY in RC mode"

On Tue, Jul 19, 2022 at 11:06:26PM +0300, Dmitry Baryshkov wrote:
> Call phy_set_mode_ext() to notify the PHY driver that the PHY is being
> used in the EP mode.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  drivers/pci/controller/dwc/pcie-qcom-ep.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> index ec99116ad05c..11f887a2f7c4 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> @@ -240,6 +240,10 @@ static int qcom_pcie_enable_resources(struct qcom_pcie_ep *pcie_ep)
>  	if (ret)
>  		goto err_disable_clk;
>  
> +	ret = phy_set_mode_ext(pcie->phy, PHY_MODE_PCIE, 1);
> +	if (ret)
> +		goto err_phy_exit;
> +
>  	ret = phy_power_on(pcie_ep->phy);
>  	if (ret)
>  		goto err_phy_exit;
> -- 
> 2.35.1
> 

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

Re: [RFC PATCH 0/4] PCI: qcom: support using the same PHY for both RC and EP

[Manivannan Sadhasivam]

On Tue, Jul 19, 2022 at 11:06:22PM +0300, Dmitry Baryshkov wrote:
> Programming of QMP PCIe PHYs slightly differs between RC and EP modes.
> 
> Currently both qcom and qcom-ep PCIe controllers setup the PHY in the
> default mode, making it impossible to select at runtime whether the PHY
> should be running in RC or in EP modes. Usually this is not an issue,
> since for most devices only the RC mode is used, while for some (SDX55)
> the EP mode is used without support for working as the RC.
> 

SDX55 could work in RC mode also. Support is on the way.

> Some of the Qualcomm platforms would still benefit from being able to
> switch between RC and EP depending on the driver being used. While it is
> possible to use different compat strings for the PHY depending on the
> mode, it seems like an incorrect approach, since the PHY doesn't differ
> between usecases. It's the PCIe controller, who should decide how to
> configure the PHY.
> 
> This patch series implements the ability to select between RC and EP
> modes, by allowing the PCIe QMP PHY driver to switch between
> programming tables.
> 

This is really nice! On the case of SDX55, there is a single PHY and PCIe
controller that is being used as both RC and EP depending on the usecase. While
it makes sense to use a different PCIe node based on usecase, it does not for
the PHY. So the runtime switch is a neat way of handling the differences.

I've provided my review for the patches. But for the next iteration, you could
remove the RFC tag.

Also, please mention the dependency of the series in the cover letter if any.
Like this one depends on your previous PHY cleanup series. It will help
maintainers while picking the patches.

Thanks a lot for the series!

Regards,
Mani

> Dmitry Baryshkov (4):
>   phy: qcom-qmp-pcie: split register tables into primary and secondary
>     part
>   phy: qcom-qmp-pcie: suppor separate tables for EP mode
>   PCI: qcom: call phy_set_mode_ext()
>   PCI: qcom-ep: call phy_set_mode_ext()
> 
>  drivers/pci/controller/dwc/pcie-qcom-ep.c |   4 +
>  drivers/pci/controller/dwc/pcie-qcom.c    |   4 +
>  drivers/phy/qualcomm/phy-qcom-qmp-pcie.c  | 155 ++++++++++++----------
>  3 files changed, 96 insertions(+), 67 deletions(-)
> 
> -- 
> 2.35.1
> 

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

Re: [RFC PATCH 0/4] PCI: qcom: support using the same PHY for both RC and EP

[Dmitry Baryshkov]

On Thu, 21 Jul 2022 at 13:15, Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> On Tue, Jul 19, 2022 at 11:06:22PM +0300, Dmitry Baryshkov wrote:
> > Programming of QMP PCIe PHYs slightly differs between RC and EP modes.
> >
> > Currently both qcom and qcom-ep PCIe controllers setup the PHY in the
> > default mode, making it impossible to select at runtime whether the PHY
> > should be running in RC or in EP modes. Usually this is not an issue,
> > since for most devices only the RC mode is used, while for some (SDX55)
> > the EP mode is used without support for working as the RC.
> >
>
> SDX55 could work in RC mode also. Support is on the way.
>
> > Some of the Qualcomm platforms would still benefit from being able to
> > switch between RC and EP depending on the driver being used. While it is
> > possible to use different compat strings for the PHY depending on the
> > mode, it seems like an incorrect approach, since the PHY doesn't differ
> > between usecases. It's the PCIe controller, who should decide how to
> > configure the PHY.
> >
> > This patch series implements the ability to select between RC and EP
> > modes, by allowing the PCIe QMP PHY driver to switch between
> > programming tables.
> >
>
> This is really nice! On the case of SDX55, there is a single PHY and PCIe
> controller that is being used as both RC and EP depending on the usecase. While
> it makes sense to use a different PCIe node based on usecase, it does not for
> the PHY. So the runtime switch is a neat way of handling the differences.
>
> I've provided my review for the patches. But for the next iteration, you could
> remove the RFC tag.

Thanks for the review!

> Also, please mention the dependency of the series in the cover letter if any.
> Like this one depends on your previous PHY cleanup series. It will help
> maintainers while picking the patches.

Ack. However the PHY series are already part of phy/next, thus I
omitted mentioning them.

>
> Thanks a lot for the series!
>
> Regards,
> Mani
>
> > Dmitry Baryshkov (4):
> >   phy: qcom-qmp-pcie: split register tables into primary and secondary
> >     part
> >   phy: qcom-qmp-pcie: suppor separate tables for EP mode
> >   PCI: qcom: call phy_set_mode_ext()
> >   PCI: qcom-ep: call phy_set_mode_ext()
> >
> >  drivers/pci/controller/dwc/pcie-qcom-ep.c |   4 +
> >  drivers/pci/controller/dwc/pcie-qcom.c    |   4 +
> >  drivers/phy/qualcomm/phy-qcom-qmp-pcie.c  | 155 ++++++++++++----------
> >  3 files changed, 96 insertions(+), 67 deletions(-)
> >
> > --
> > 2.35.1
> >
>
> --
> மணிவண்ணன் சதாசிவம்

-- 
With best wishes
Dmitry