[PATCH 0/7] Add support for IPQ8074 PCIe phy and controller

[PATCH 0/7] Add support for IPQ8074 PCIe phy and controller

[Varadarajan Narayanan]

Add definitions required to enable QMP phy support for IPQ8074.

Add support for the IPQ8074 PCIe controller.  IPQ8074 supports
Gen 1/2, one lane, two PCIe root complex with support for MSI and
legacy interrupts, and it conforms to PCI Express Base 2.1
specification.

Varadarajan Narayanan (7):
  dt-bindings: phy: qmp: Add output-clock-names
  dt-bindings: phy: qmp: Add support for QMP phy in IPQ8074
  phy: qcom-qmp: Fix phy pipe clock name
  phy: qcom-qmp: Handle unavailable registers
  phy: qcom-qmp: Add support for IPQ8074
  dt-bindings: pci: qcom: Add support for IPQ8074
  PCI: dwc: qcom: Add support for IPQ8074 PCIe controller

 .../devicetree/bindings/pci/qcom,pcie.txt          |  67 ++++++
 .../devicetree/bindings/phy/qcom-qmp-phy.txt       |  31 +++
 drivers/pci/dwc/pcie-qcom.c                        | 259 +++++++++++++++++++++
 drivers/phy/qualcomm/phy-qcom-qmp.c                | 186 +++++++++++++--
 4 files changed, 522 insertions(+), 21 deletions(-)

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/7] Add support for IPQ8074 PCIe phy and controller

[Varadarajan Narayanan]

Add definitions required to enable QMP phy support for IPQ8074.

Add support for the IPQ8074 PCIe controller.  IPQ8074 supports
Gen 1/2, one lane, two PCIe root complex with support for MSI and
legacy interrupts, and it conforms to PCI Express Base 2.1
specification.

Varadarajan Narayanan (7):
  dt-bindings: phy: qmp: Add output-clock-names
  dt-bindings: phy: qmp: Add support for QMP phy in IPQ8074
  phy: qcom-qmp: Fix phy pipe clock name
  phy: qcom-qmp: Handle unavailable registers
  phy: qcom-qmp: Add support for IPQ8074
  dt-bindings: pci: qcom: Add support for IPQ8074
  PCI: dwc: qcom: Add support for IPQ8074 PCIe controller

 .../devicetree/bindings/pci/qcom,pcie.txt          |  67 ++++++
 .../devicetree/bindings/phy/qcom-qmp-phy.txt       |  31 +++
 drivers/pci/dwc/pcie-qcom.c                        | 259 +++++++++++++++++++++
 drivers/phy/qualcomm/phy-qcom-qmp.c                | 186 +++++++++++++--
 4 files changed, 522 insertions(+), 21 deletions(-)

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 1/7] dt-bindings: phy: qmp: Add output-clock-names

[Varadarajan Narayanan]

The phy outputs a clock that will act as the parent for
the phy's pipe clock. Add the name of this clock to the
lane's DT node.

Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
---
 Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
index e11c563..5d7a51f 100644
--- a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
+++ b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
@@ -60,6 +60,8 @@ Required properties for child node:
 	   one for each entry in clock-names.
  - clock-names: Must contain following for pcie and usb qmp phys:
 		 "pipe<lane-number>" for pipe clock specific to each lane.
+ - clock-output-names: Name of the phy clock that will be the parent for
+		       the above pipe clock.
 
  - resets: a list of phandles and reset controller specifier pairs,
 	   one for each entry in reset-names.
@@ -96,6 +98,7 @@ Example:
 
 			clocks = <&gcc GCC_PCIE_0_PIPE_CLK>;
 			clock-names = "pipe0";
+			clock-output-names = "pcie_0_pipe_clk_src";
 			resets = <&gcc GCC_PCIE_0_PHY_BCR>;
 			reset-names = "lane0";
 		};
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 2/7] dt-bindings: phy: qmp: Add support for QMP phy in IPQ8074

[Varadarajan Narayanan]

IPQ8074 uses QMP phy controller that provides support to PCIe and
USB. Adding dt binding information for the same.

Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
---
 .../devicetree/bindings/phy/qcom-qmp-phy.txt       | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
index 5d7a51f..80d766b 100644
--- a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
+++ b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
@@ -6,6 +6,7 @@ controllers on Qualcomm chipsets, such as, PCIe, UFS, and USB.
 
 Required properties:
  - compatible: compatible list, contains:
+	       "qcom,ipq8074-qmp-pcie-phy" for PCIe phy on IPQ8074
 	       "qcom,msm8996-qmp-pcie-phy" for 14nm PCIe phy on msm8996,
 	       "qcom,msm8996-qmp-usb3-phy" for 14nm USB3 phy on msm8996.
 
@@ -107,3 +108,30 @@ Example:
 		...
 		...
 	};
+
+	phy@86000 {
+		compatible = "qcom,ipq8074-qmp-pcie-phy";
+		reg = <0x86000 0x1000>;
+
+		#clock-cells = <1>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		resets = <&gcc GCC_PCIE0_PHY_BCR>,
+			 <&gcc GCC_PCIE0PHY_PHY_BCR>;
+		reset-names = "phy",
+			      "phy_phy";
+
+		status = "ok";
+
+		pciephy_0: lane@86000 {
+			reg = <0x86200 0x130>,
+				<0x86400 0x200>,
+				<0x86800 0x1f8>;
+			#phy-cells = <0>;
+			clocks = <&gcc GCC_PCIE0_PIPE_CLK>;
+			clock-names = "pipe0";
+			clock-output-names = "pcie20_phy0_pipe_clk";
+		};
+	};
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 3/7] phy: qcom-qmp: Fix phy pipe clock name

[Varadarajan Narayanan]

Presently, the phy pipe clock's name is assumed to be either
usb3_phy_pipe_clk_src or pcie_XX_pipe_clk_src (where XX is the
phy lane's number). However, this will not work if an SoC has
more than one instance of the phy. Hence, instead of assuming
the name of the clock, fetch it from the DT.

Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
---
 drivers/phy/qualcomm/phy-qcom-qmp.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c
index 78ca628..97020ec 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
@@ -925,20 +925,13 @@ static int qcom_qmp_phy_clk_init(struct device *dev)
  *    clk  |   +-------+   |                   +-----+
  *         +---------------+
  */
-static int phy_pipe_clk_register(struct qcom_qmp *qmp, int id)
+static int phy_pipe_clk_register(struct qcom_qmp *qmp, const char *clk_name)
 {
-	char name[24];
 	struct clk_fixed_rate *fixed;
 	struct clk_init_data init = { };
 
-	switch (qmp->cfg->type) {
-	case PHY_TYPE_USB3:
-		snprintf(name, sizeof(name), "usb3_phy_pipe_clk_src");
-		break;
-	case PHY_TYPE_PCIE:
-		snprintf(name, sizeof(name), "pcie_%d_pipe_clk_src", id);
-		break;
-	default:
+	if ((qmp->cfg->type != PHY_TYPE_USB3) &&
+	    (qmp->cfg->type != PHY_TYPE_PCIE)) {
 		/* not all phys register pipe clocks, so return success */
 		return 0;
 	}
@@ -947,7 +940,7 @@ static int phy_pipe_clk_register(struct qcom_qmp *qmp, int id)
 	if (!fixed)
 		return -ENOMEM;
 
-	init.name = name;
+	init.name = clk_name;
 	init.ops = &clk_fixed_rate_ops;
 
 	/* controllers using QMP phys use 125MHz pipe clock interface */
@@ -1110,6 +1103,8 @@ static int qcom_qmp_phy_probe(struct platform_device *pdev)
 
 	id = 0;
 	for_each_available_child_of_node(dev->of_node, child) {
+		const char *clk_name;
+
 		/* Create per-lane phy */
 		ret = qcom_qmp_phy_create(dev, child, id);
 		if (ret) {
@@ -1118,11 +1113,20 @@ static int qcom_qmp_phy_probe(struct platform_device *pdev)
 			return ret;
 		}
 
+		ret = of_property_read_string(child, "clock-output-names",
+							&clk_name);
+		if (ret) {
+			dev_err(dev,
+				"failed to get clock-output-names for lane%d phy, %d\n",
+				id, ret);
+			return ret;
+		}
+
 		/*
 		 * Register the pipe clock provided by phy.
 		 * See function description to see details of this pipe clock.
 		 */
-		ret = phy_pipe_clk_register(qmp, id);
+		ret = phy_pipe_clk_register(qmp, clk_name);
 		if (ret) {
 			dev_err(qmp->dev,
 				"failed to register pipe clock source\n");
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 4/7] phy: qcom-qmp: Handle unavailable registers

[Varadarajan Narayanan]

In some implementations of the QMP phy, some registers might not
be present. Provide a way identify such registers and not access
those registers.

Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
---
 drivers/phy/qualcomm/phy-qcom-qmp.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c
index 97020ec..000ad1c 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
@@ -187,6 +187,8 @@ struct qmp_phy_init_tbl {
 		.in_layout = 1,		\
 	}
 
+#define QPHY_REG_INVAL		0xffffffffu
+
 /* set of registers with offsets different per-PHY */
 enum qphy_reg_layout {
 	/* Common block control registers */
@@ -676,15 +678,18 @@ static int qcom_qmp_phy_com_init(struct qcom_qmp *qmp)
 		qphy_setbits(serdes, cfg->regs[QPHY_COM_START_CONTROL],
 			     SERDES_START | PCS_START);
 
-		status = serdes + cfg->regs[QPHY_COM_PCS_READY_STATUS];
-		mask = cfg->mask_com_pcs_ready;
-
-		ret = readl_poll_timeout(status, val, (val & mask), 10,
-					 PHY_INIT_COMPLETE_TIMEOUT);
-		if (ret) {
-			dev_err(qmp->dev,
-				"phy common block init timed-out\n");
-			goto err_com_init;
+		if (cfg->regs[QPHY_COM_PCS_READY_STATUS] != QPHY_REG_INVAL) {
+			status = serdes + cfg->regs[QPHY_COM_PCS_READY_STATUS];
+			mask = cfg->mask_com_pcs_ready;
+
+			ret = readl_poll_timeout(status, val, (val & mask), 10,
+						 PHY_INIT_COMPLETE_TIMEOUT);
+			if (ret) {
+				dev_err(qmp->dev,
+					"%s: phy common block init timed-out\n",
+					__func__);
+				goto err_com_init;
+			}
 		}
 	}
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 5/7] phy: qcom-qmp: Add support for IPQ8074

[Varadarajan Narayanan]

Add definitions required to enable QMP phy support for IPQ8074.

Signed-off-by: smuthayy <smuthayy@codeaurora.org>
Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
---
 drivers/phy/qualcomm/phy-qcom-qmp.c | 135 ++++++++++++++++++++++++++++++++++++
 1 file changed, 135 insertions(+)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c
index 000ad1c..9019f66 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
@@ -59,6 +59,7 @@
 #define QSERDES_COM_PLL_RCTRL_MODE1			0x088
 #define QSERDES_COM_PLL_CCTRL_MODE0			0x090
 #define QSERDES_COM_PLL_CCTRL_MODE1			0x094
+#define QSERDES_COM_BIAS_EN_CTRL_BY_PSM			0x0a8
 #define QSERDES_COM_SYSCLK_EN_SEL			0x0ac
 #define QSERDES_COM_RESETSM_CNTRL			0x0b4
 #define QSERDES_COM_RESTRIM_CTRL			0x0bc
@@ -143,6 +144,11 @@
 #define QPHY_LOCK_DETECT_CONFIG3			0x88
 #define QPHY_PWRUP_RESET_DLY_TIME_AUXCLK		0xa0
 #define QPHY_LP_WAKEUP_DLY_TIME_AUXCLK			0xa4
+#define QPHY_PLL_LOCK_CHK_DLY_TIME_AUXCLK_LSB		0x1A8
+#define QPHY_OSC_DTCT_ACTIONS				0x1AC
+#define QPHY_RX_SIGDET_LVL				0x1D8
+#define QPHY_L1SS_WAKEUP_DLY_TIME_AUXCLK_LSB		0x1DC
+#define QPHY_L1SS_WAKEUP_DLY_TIME_AUXCLK_MSB		0x1E0
 
 /* QPHY_SW_RESET bit */
 #define SW_RESET				BIT(0)
@@ -224,6 +230,17 @@ enum qphy_reg_layout {
 	[QPHY_PCS_READY_STATUS]		= 0x174,
 };
 
+static const unsigned int ipq8074_pciephy_regs_layout[] = {
+	[QPHY_COM_SW_RESET]		= 0x800,
+	[QPHY_COM_POWER_DOWN_CONTROL]	= 0x804,
+	[QPHY_COM_START_CONTROL]	= 0x808,
+	[QPHY_COM_PCS_READY_STATUS]	= QPHY_REG_INVAL,
+	[QPHY_PLL_LOCK_CHK_DLY_TIME]	= 0xa8,
+	[QPHY_SW_RESET]			= 0x00,
+	[QPHY_START_CTRL]		= 0x08,
+	[QPHY_PCS_READY_STATUS]		= 0x174,
+};
+
 static const unsigned int usb3phy_regs_layout[] = {
 	[QPHY_FLL_CNTRL1]		= 0xc0,
 	[QPHY_FLL_CNTRL2]		= 0xc4,
@@ -582,6 +599,121 @@ static inline void qphy_clrbits(void __iomem *base, u32 offset, u32 val)
 	.mask_pcs_ready		= PHYSTATUS,
 };
 
+static const struct qmp_phy_init_tbl ipq8074_pcie_serdes_tbl[] = {
+	QMP_PHY_INIT_CFG(QSERDES_COM_BIAS_EN_CLKBUFLR_EN, 0x18),
+	QMP_PHY_INIT_CFG(QSERDES_COM_CLK_ENABLE1, 0x10),
+	QMP_PHY_INIT_CFG(QSERDES_COM_BG_TRIM, 0xf),
+	QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP_EN, 0x1),
+	QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_MAP, 0x0),
+	QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_TIMER1, 0x1f),
+	QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_TIMER2, 0x3f),
+	QMP_PHY_INIT_CFG(QSERDES_COM_CMN_CONFIG, 0x6),
+	QMP_PHY_INIT_CFG(QSERDES_COM_PLL_IVCO, 0xf),
+	QMP_PHY_INIT_CFG(QSERDES_COM_HSCLK_SEL, 0x0),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SVS_MODE_CLK_SEL, 0x1),
+	QMP_PHY_INIT_CFG(QSERDES_COM_CORE_CLK_EN, 0x20),
+	QMP_PHY_INIT_CFG(QSERDES_COM_CORECLK_DIV, 0xa),
+	QMP_PHY_INIT_CFG(QSERDES_COM_RESETSM_CNTRL, 0x20),
+	QMP_PHY_INIT_CFG(QSERDES_COM_BG_TIMER, 0xa),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SYSCLK_EN_SEL, 0xa),
+	QMP_PHY_INIT_CFG(QSERDES_COM_DEC_START_MODE0, 0x82),
+	QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START3_MODE0, 0x3),
+	QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START2_MODE0, 0x55),
+	QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START1_MODE0, 0x55),
+	QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP3_MODE0, 0x0),
+	QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP2_MODE0, 0xD),
+	QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP1_MODE0, 0xD04),
+	QMP_PHY_INIT_CFG(QSERDES_COM_CLK_SELECT, 0x33),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SYS_CLK_CTRL, 0x2),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SYSCLK_BUF_ENABLE, 0x1f),
+	QMP_PHY_INIT_CFG(QSERDES_COM_CP_CTRL_MODE0, 0xb),
+	QMP_PHY_INIT_CFG(QSERDES_COM_PLL_RCTRL_MODE0, 0x16),
+	QMP_PHY_INIT_CFG(QSERDES_COM_PLL_CCTRL_MODE0, 0x28),
+	QMP_PHY_INIT_CFG(QSERDES_COM_INTEGLOOP_GAIN1_MODE0, 0x0),
+	QMP_PHY_INIT_CFG(QSERDES_COM_INTEGLOOP_GAIN0_MODE0, 0x80),
+	QMP_PHY_INIT_CFG(QSERDES_COM_BIAS_EN_CTRL_BY_PSM, 0x1),
+	QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_CTRL, 0xa),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_EN_CENTER, 0x1),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_PER1, 0x31),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_PER2, 0x1),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_ADJ_PER1, 0x2),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_ADJ_PER2, 0x0),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_STEP_SIZE1, 0x2f),
+	QMP_PHY_INIT_CFG(QSERDES_COM_SSC_STEP_SIZE2, 0x19),
+	QMP_PHY_INIT_CFG(QSERDES_COM_CLK_EP_DIV, 0x19),
+	QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_CNTRL, 0x7),
+};
+
+static const struct qmp_phy_init_tbl ipq8074_pcie_tx_tbl[] = {
+	QMP_PHY_INIT_CFG(QSERDES_TX_HIGHZ_TRANSCEIVEREN_BIAS_DRVR_EN, 0x45),
+	QMP_PHY_INIT_CFG(QSERDES_TX_LANE_MODE, 0x6),
+	QMP_PHY_INIT_CFG(QSERDES_TX_RES_CODE_LANE_OFFSET, 0x2),
+	QMP_PHY_INIT_CFG(QSERDES_TX_RCV_DETECT_LVL_2, 0x12),
+};
+
+static const struct qmp_phy_init_tbl ipq8074_pcie_rx_tbl[] = {
+	QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_ENABLES, 0x1c),
+	QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_DEGLITCH_CNTRL, 0x14),
+	QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL2, 0x1),
+	QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL3, 0x0),
+	QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL4, 0xdb),
+	QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_SO_SATURATION_AND_ENABLE, 0x4b),
+	QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_SO_GAIN, 0x4),
+	QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_SO_GAIN_HALF, 0x4),
+};
+
+static const struct qmp_phy_init_tbl ipq8074_pcie_pcs_tbl[] = {
+	QMP_PHY_INIT_CFG(QPHY_ENDPOINT_REFCLK_DRIVE, 0x4),
+	QMP_PHY_INIT_CFG(QPHY_OSC_DTCT_ACTIONS, 0x0),
+	QMP_PHY_INIT_CFG(QPHY_PWRUP_RESET_DLY_TIME_AUXCLK, 0x40),
+	QMP_PHY_INIT_CFG(QPHY_L1SS_WAKEUP_DLY_TIME_AUXCLK_MSB, 0x0),
+	QMP_PHY_INIT_CFG(QPHY_L1SS_WAKEUP_DLY_TIME_AUXCLK_LSB, 0x40),
+	QMP_PHY_INIT_CFG(QPHY_PLL_LOCK_CHK_DLY_TIME_AUXCLK_LSB, 0x0),
+	QMP_PHY_INIT_CFG(QPHY_LP_WAKEUP_DLY_TIME_AUXCLK, 0x40),
+	QMP_PHY_INIT_CFG_L(QPHY_PLL_LOCK_CHK_DLY_TIME, 0x73),
+	QMP_PHY_INIT_CFG(QPHY_RX_SIGDET_LVL, 0x99),
+	QMP_PHY_INIT_CFG(QPHY_TXDEEMPH_M6DB_V0, 0x15),
+	QMP_PHY_INIT_CFG(QPHY_TXDEEMPH_M3P5DB_V0, 0xe),
+	QMP_PHY_INIT_CFG_L(QPHY_SW_RESET, 0x0),
+	QMP_PHY_INIT_CFG_L(QPHY_START_CTRL, 0x3),
+};
+
+/* list of resets */
+static const char * const ipq8074_pciephy_reset_l[] = {
+	"phy", "phy_phy",
+};
+
+static const struct qmp_phy_cfg ipq8074_pciephy_cfg = {
+	.type			= PHY_TYPE_PCIE,
+	.nlanes			= 1,
+
+	.serdes_tbl		= ipq8074_pcie_serdes_tbl,
+	.serdes_tbl_num		= ARRAY_SIZE(ipq8074_pcie_serdes_tbl),
+	.tx_tbl			= ipq8074_pcie_tx_tbl,
+	.tx_tbl_num		= ARRAY_SIZE(ipq8074_pcie_tx_tbl),
+	.rx_tbl			= ipq8074_pcie_rx_tbl,
+	.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		= NULL,
+	.num_clks		= 0,
+	.reset_list		= ipq8074_pciephy_reset_l,
+	.num_resets		= ARRAY_SIZE(ipq8074_pciephy_reset_l),
+	.vreg_list		= NULL,
+	.num_vregs		= 0,
+	.regs			= ipq8074_pciephy_regs_layout,
+
+	.start_ctrl		= SERDES_START | PCS_START,
+	.pwrdn_ctrl		= SW_PWRDN | REFCLK_DRV_DSBL,
+	.mask_pcs_ready		= PHYSTATUS,
+
+	.has_phy_com_ctrl	= true,
+	.has_lane_rst		= false,
+	.has_pwrdn_delay	= true,
+	.pwrdn_delay_min	= 995,		/* us */
+	.pwrdn_delay_max	= 1005,		/* us */
+};
+
 static void qcom_qmp_phy_configure(void __iomem *base,
 				   const unsigned int *regs,
 				   const struct qmp_phy_init_tbl tbl[],
@@ -1047,6 +1179,9 @@ int qcom_qmp_phy_create(struct device *dev, struct device_node *np, int id)
 	}, {
 		.compatible = "qcom,msm8996-qmp-usb3-phy",
 		.data = &msm8996_usb3phy_cfg,
+	}, {
+		.compatible = "qcom,ipq8074-qmp-pcie-phy",
+		.data = &ipq8074_pciephy_cfg,
 	},
 	{ },
 };
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 6/7] dt-bindings: pci: qcom: Add support for IPQ8074

[Varadarajan Narayanan]

Add support for the IPQ8074 PCIe controller.  IPQ8074 supports Gen 1/2, one
lane, two PCIe root complex with support for MSI and legacy interrupts, and
it conforms to PCI Express Base 2.1 specification.

Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
---
 .../devicetree/bindings/pci/qcom,pcie.txt          | 67 ++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
index 9d418b7..643bcc2 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
@@ -9,6 +9,7 @@
 			- "qcom,pcie-apq8084" for apq8084
 			- "qcom,pcie-msm8996" for msm8996 or apq8096
 			- "qcom,pcie-ipq4019" for ipq4019
+			- "qcom,pcie-ipq8074" for ipq8074
 
 - reg:
 	Usage: required
@@ -261,3 +262,69 @@
 		pinctrl-0 = <&pcie0_pins_default>;
 		pinctrl-names = "default";
 	};
+
+* Example for ipq8074
+	pcie0: pci@20000000 {
+		compatible = "qcom,pcie-ipq8074";
+		reg =  <0x20000000 0xf1d
+			0x20000F20 0xa8
+			0x80000 0x2000
+			0x20100000 0x1000>;
+		reg-names = "dbi", "elbi", "parf", "config";
+		device_type = "pci";
+		linux,pci-domain = <0>;
+		bus-range = <0x00 0xff>;
+		num-lanes = <1>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+
+		phys = <&pciephy_0>;
+		phy-names = "pciephy";
+
+		ranges = <0x81000000 0 0x20200000 0x20200000
+			  0 0x00100000   /* downstream I/O */
+			  0x82000000 0 0x20300000 0x20300000
+			  0 0x00d00000>; /* non-prefetchable memory */
+
+		interrupts = <0 52 0>;
+		interrupt-names = "msi";
+		#interrupt-cells = <1>;
+		interrupt-map-mask = <0 0 0 0x7>;
+		interrupt-map = <0 0 0 1 &intc 0 75
+				 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
+				<0 0 0 2 &intc 0 78
+				 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
+				<0 0 0 3 &intc 0 79
+				 IRQ_TYPE_LEVEL_HIGH>, /* int_c */
+				<0 0 0 4 &intc 0 83
+				 IRQ_TYPE_LEVEL_HIGH>; /* int_d */
+
+		clocks =	<&gcc GCC_SYS_NOC_PCIE0_AXI_CLK>,
+				<&gcc GCC_PCIE0_AXI_M_CLK>,
+				<&gcc GCC_PCIE0_AXI_S_CLK>,
+				<&gcc GCC_PCIE0_AHB_CLK>,
+				<&gcc GCC_PCIE0_AUX_CLK>;
+		clock-names =	"sys_noc",
+				"axi_m",
+				"axi_s",
+				"ahb",
+				"aux";
+
+		resets = <&gcc GCC_PCIE0_PIPE_ARES>,
+			 <&gcc GCC_PCIE0_SLEEP_ARES>,
+			 <&gcc GCC_PCIE0_CORE_STICKY_ARES>,
+			 <&gcc GCC_PCIE0_AXI_MASTER_ARES>,
+			 <&gcc GCC_PCIE0_AXI_SLAVE_ARES>,
+			 <&gcc GCC_PCIE0_AHB_ARES>,
+			 <&gcc GCC_PCIE0_AXI_MASTER_STICKY_ARES>;
+		reset-names = "pipe",
+			      "sleep",
+			      "sticky",
+			      "axi_m",
+			      "axi_s",
+			      "ahb",
+			      "axi_m_sticky";
+
+		perst-gpio = <&tlmm 58 1>;
+		status = "disabled";
+	};
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 7/7] PCI: dwc: qcom: Add support for IPQ8074 PCIe controller

[Varadarajan Narayanan]

Add support for the IPQ8074 PCIe controller.  IPQ8074 supports
Gen 1/2, one lane, two PCIe root complex with support for MSI and
legacy interrupts, and it conforms to PCI Express Base 2.1
specification.

The core init is the similar to the existing SoC, however the
clocks and reset lines differ.

Signed-off-by: smuthayy <smuthayy@codeaurora.org>
Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
---
 drivers/pci/dwc/pcie-qcom.c | 259 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 259 insertions(+)

diff --git a/drivers/pci/dwc/pcie-qcom.c b/drivers/pci/dwc/pcie-qcom.c
index d15657d..c1fa356 100644
--- a/drivers/pci/dwc/pcie-qcom.c
+++ b/drivers/pci/dwc/pcie-qcom.c
@@ -37,6 +37,20 @@
 #include "pcie-designware.h"
 
 #define PCIE20_PARF_SYS_CTRL			0x00
+#define MST_WAKEUP_EN				BIT(13)
+#define SLV_WAKEUP_EN				BIT(12)
+#define MSTR_ACLK_CGC_DIS			BIT(10)
+#define SLV_ACLK_CGC_DIS			BIT(9)
+#define CORE_CLK_CGC_DIS			BIT(6)
+#define AUX_PWR_DET				BIT(4)
+#define L23_CLK_RMV_DIS				BIT(2)
+#define L1_CLK_RMV_DIS				BIT(1)
+
+#define PCIE20_COMMAND_STATUS			0x04
+#define CMD_BME_VAL				0x4
+#define PCIE20_DEVICE_CONTROL2_STATUS2		0x98
+#define PCIE_CAP_CPL_TIMEOUT_DISABLE		0x10
+
 #define PCIE20_PARF_PHY_CTRL			0x40
 #define PCIE20_PARF_PHY_REFCLK			0x4C
 #define PCIE20_PARF_DBI_BASE_ADDR		0x168
@@ -58,9 +72,22 @@
 #define CFG_BRIDGE_SB_INIT			BIT(0)
 
 #define PCIE20_CAP				0x70
+#define PCIE20_CAP_LINK_CAPABILITIES		(PCIE20_CAP + 0xC)
+#define PCIE20_CAP_LINK_1			(PCIE20_CAP + 0x14)
+#define PCIE_CAP_LINK1_VAL			0x2fd7f
+
+#define PCIE20_PARF_Q2A_FLUSH			0x1AC
+
+#define PCIE20_MISC_CONTROL_1_REG		0x8BC
+#define DBI_RO_WR_EN				1
 
 #define PERST_DELAY_US				1000
 
+#define AXI_CLK_RATE				200000000
+
+#define PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE	0x358
+#define SLV_ADDR_SPACE_SZ                       0x10000000
+
 struct qcom_pcie_resources_v0 {
 	struct clk *iface_clk;
 	struct clk *core_clk;
@@ -110,11 +137,26 @@ struct qcom_pcie_resources_v3 {
 	struct reset_control *phy_ahb_reset;
 };
 
+struct qphy_reset {
+	struct reset_control	*rst;
+	char			*name;
+};
+
+struct qcom_pcie_resources_v4 {
+	struct clk *sys_noc_clk;
+	struct clk *axi_m_clk;
+	struct clk *axi_s_clk;
+	struct clk *ahb_clk;
+	struct clk *aux_clk;
+	struct qphy_reset rst[7];
+};
+
 union qcom_pcie_resources {
 	struct qcom_pcie_resources_v0 v0;
 	struct qcom_pcie_resources_v1 v1;
 	struct qcom_pcie_resources_v2 v2;
 	struct qcom_pcie_resources_v3 v3;
+	struct qcom_pcie_resources_v4 v4;
 };
 
 struct qcom_pcie;
@@ -139,6 +181,16 @@ struct qcom_pcie {
 
 #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
 
+static inline void
+writel_masked(void __iomem *addr, u32 clear_mask, u32 set_mask)
+{
+	u32 val = readl(addr);
+
+	val &= ~clear_mask;
+	val |= set_mask;
+	writel(val, addr);
+}
+
 static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
 {
 	gpiod_set_value(pcie->reset, 1);
@@ -884,6 +936,205 @@ static int qcom_pcie_init_v3(struct qcom_pcie *pcie)
 	return ret;
 }
 
+static int qcom_pcie_get_resources_v4(struct qcom_pcie *pcie)
+{
+	struct qcom_pcie_resources_v4 *res = &pcie->res.v4;
+	struct dw_pcie *pci = pcie->pci;
+	struct device *dev = pci->dev;
+	int i;
+
+	res->sys_noc_clk = devm_clk_get(dev, "sys_noc");
+	if (IS_ERR(res->sys_noc_clk))
+		return PTR_ERR(res->sys_noc_clk);
+
+	res->axi_m_clk = devm_clk_get(dev, "axi_m");
+	if (IS_ERR(res->axi_m_clk))
+		return PTR_ERR(res->axi_m_clk);
+
+	res->axi_s_clk = devm_clk_get(dev, "axi_s");
+	if (IS_ERR(res->axi_s_clk))
+		return PTR_ERR(res->axi_s_clk);
+
+	res->ahb_clk = devm_clk_get(dev, "ahb");
+	if (IS_ERR(res->ahb_clk))
+		return PTR_ERR(res->ahb_clk);
+
+	res->aux_clk = devm_clk_get(dev, "aux");
+	if (IS_ERR(res->aux_clk))
+		return PTR_ERR(res->aux_clk);
+
+	res->rst[0].name = "axi_m";
+	res->rst[1].name = "axi_s";
+	res->rst[2].name = "pipe";
+	res->rst[3].name = "axi_m_sticky";
+	res->rst[4].name = "sticky";
+	res->rst[5].name = "ahb";
+	res->rst[6].name = "sleep";
+
+	for (i = 0; i < ARRAY_SIZE(res->rst); i++) {
+		res->rst[i].rst = devm_reset_control_get(dev, res->rst[i].name);
+		if (IS_ERR(res->rst[i].rst))
+			return PTR_ERR(res->rst[i].rst);
+	}
+
+	return 0;
+}
+
+static void qcom_pcie_deinit_v4(struct qcom_pcie *pcie)
+{
+	struct qcom_pcie_resources_v4 *res = &pcie->res.v4;
+
+	clk_disable_unprepare(res->sys_noc_clk);
+	clk_disable_unprepare(res->axi_m_clk);
+	clk_disable_unprepare(res->axi_s_clk);
+	clk_disable_unprepare(res->ahb_clk);
+	clk_disable_unprepare(res->aux_clk);
+}
+
+static int qcom_pcie_enable_resources_v4(struct qcom_pcie *pcie)
+{
+	struct qcom_pcie_resources_v4 *res = &pcie->res.v4;
+	struct dw_pcie *pci = pcie->pci;
+	struct device *dev = pci->dev;
+	int ret;
+
+	ret = clk_prepare_enable(res->sys_noc_clk);
+	if (ret) {
+		dev_err(dev, "cannot prepare/enable core clock\n");
+		return ret;
+	}
+
+	ret = clk_prepare_enable(res->axi_m_clk);
+	if (ret) {
+		dev_err(dev, "cannot prepare/enable core clock\n");
+		goto err_clk_axi_m;
+	}
+
+	ret = clk_set_rate(res->axi_m_clk, AXI_CLK_RATE);
+	if (ret) {
+		dev_err(dev, "MClk rate set failed (%d)\n", ret);
+		goto err_clk_axi_m;
+	}
+
+	ret = clk_prepare_enable(res->axi_s_clk);
+	if (ret) {
+		dev_err(dev, "cannot prepare/enable axi slave clock\n");
+		goto err_clk_axi_s;
+	}
+
+	ret = clk_set_rate(res->axi_s_clk, AXI_CLK_RATE);
+	if (ret) {
+		dev_err(dev, "MClk rate set failed (%d)\n", ret);
+		goto err_clk_axi_s;
+	}
+
+	ret = clk_prepare_enable(res->ahb_clk);
+	if (ret) {
+		dev_err(dev, "cannot prepare/enable ahb clock\n");
+		goto err_clk_ahb;
+	}
+
+	ret = clk_prepare_enable(res->aux_clk);
+	if (ret) {
+		dev_err(dev, "cannot prepare/enable aux clock\n");
+		goto err_clk_aux;
+	}
+
+	udelay(1);
+
+	return 0;
+
+err_clk_aux:
+	clk_disable_unprepare(res->ahb_clk);
+err_clk_ahb:
+	clk_disable_unprepare(res->axi_s_clk);
+err_clk_axi_s:
+	clk_disable_unprepare(res->axi_m_clk);
+err_clk_axi_m:
+	clk_disable_unprepare(res->sys_noc_clk);
+
+	return ret;
+}
+
+static inline int qphy_reset_control(struct qcom_pcie *pcie,
+				     struct qphy_reset *r,
+				     bool assert)
+{
+	int ret;
+
+	if (assert)
+		ret = reset_control_assert(r->rst);
+	else
+		ret = reset_control_deassert(r->rst);
+
+	if (ret)
+		dev_err(pcie->pci->dev, "%s: reset %s failed for %s\n",
+			__func__, assert ? "assert" : "deassert", r->name);
+
+	return ret;
+}
+
+static void qcom_pcie_v4_reset(struct qcom_pcie *pcie)
+{
+	struct qcom_pcie_resources_v4 *res = &pcie->res.v4;
+	struct qphy_reset *qphy_rst = &res->rst[0];
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(res->rst); i++)
+		if (qphy_reset_control(pcie, &qphy_rst[i], true))
+			return;
+
+	usleep_range(10000, 12000); /* wait 12ms */
+
+	for (i = 0; i < ARRAY_SIZE(res->rst); i++)
+		if (qphy_reset_control(pcie, &qphy_rst[i], false))
+			return;
+
+	usleep_range(10000, 12000); /* wait 12ms */
+	wmb(); /* ensure data is written to hw register */
+}
+
+static int qcom_pcie_init_v4(struct qcom_pcie *pcie)
+{
+	struct dw_pcie *pci = pcie->pci;
+	int ret;
+
+	qcom_pcie_v4_reset(pcie);
+	qcom_ep_reset_assert(pcie);
+
+	ret = qcom_pcie_enable_resources_v4(pcie);
+	if (ret)
+		return ret;
+
+	writel(SLV_ADDR_SPACE_SZ, pcie->parf +
+					PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE);
+
+	ret = phy_power_on(pcie->phy);
+	if (ret)
+		return ret;
+
+	writel_masked(pcie->parf + PCIE20_PARF_PHY_CTRL, BIT(0), 0);
+
+	writel(0, pcie->parf + PCIE20_PARF_DBI_BASE_ADDR);
+
+	writel(MST_WAKEUP_EN | SLV_WAKEUP_EN | MSTR_ACLK_CGC_DIS
+		| SLV_ACLK_CGC_DIS | CORE_CLK_CGC_DIS |
+		AUX_PWR_DET | L23_CLK_RMV_DIS | L1_CLK_RMV_DIS,
+		pcie->parf + PCIE20_PARF_SYS_CTRL);
+	writel(0, pcie->parf + PCIE20_PARF_Q2A_FLUSH);
+
+	writel(CMD_BME_VAL, pci->dbi_base + PCIE20_COMMAND_STATUS);
+	writel(DBI_RO_WR_EN, pci->dbi_base + PCIE20_MISC_CONTROL_1_REG);
+	writel(PCIE_CAP_LINK1_VAL, pci->dbi_base + PCIE20_CAP_LINK_1);
+
+	writel_masked(pci->dbi_base + PCIE20_CAP_LINK_CAPABILITIES,
+		BIT(10) | BIT(11), 0);
+	writel(PCIE_CAP_CPL_TIMEOUT_DISABLE, pci->dbi_base +
+		PCIE20_DEVICE_CONTROL2_STATUS2);
+
+	return 0;
+}
+
 static int qcom_pcie_link_up(struct dw_pcie *pci)
 {
 	u16 val = readw(pci->dbi_base + PCIE20_CAP + PCI_EXP_LNKSTA);
@@ -985,6 +1236,13 @@ static int qcom_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
 	.ltssm_enable = qcom_pcie_v2_ltssm_enable,
 };
 
+static const struct qcom_pcie_ops ops_v4 = {
+	.get_resources = qcom_pcie_get_resources_v4,
+	.init = qcom_pcie_init_v4,
+	.deinit = qcom_pcie_deinit_v4,
+	.ltssm_enable = qcom_pcie_v2_ltssm_enable,
+};
+
 static int qcom_pcie_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -1076,6 +1334,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 	{ .compatible = "qcom,pcie-apq8084", .data = &ops_v1 },
 	{ .compatible = "qcom,pcie-msm8996", .data = &ops_v2 },
 	{ .compatible = "qcom,pcie-ipq4019", .data = &ops_v3 },
+	{ .compatible = "qcom,pcie-ipq8074", .data = &ops_v4 },
 	{ }
 };
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 1/7] dt-bindings: phy: qmp: Add output-clock-names

[Rob Herring]

On Mon, Jul 17, 2017 at 05:33:57PM +0530, Varadarajan Narayanan wrote:
> The phy outputs a clock that will act as the parent for
> the phy's pipe clock. Add the name of this clock to the
> lane's DT node.
> 
> Signed-off-by: Varadarajan Narayanan <varada-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt | 3 +++
>  1 file changed, 3 insertions(+)

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/7] dt-bindings: phy: qmp: Add output-clock-names

[Rob Herring]

On Mon, Jul 17, 2017 at 05:33:57PM +0530, Varadarajan Narayanan wrote:
> The phy outputs a clock that will act as the parent for
> the phy's pipe clock. Add the name of this clock to the
> lane's DT node.
> 
> Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt | 3 +++
>  1 file changed, 3 insertions(+)

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 2/7] dt-bindings: phy: qmp: Add support for QMP phy in IPQ8074

[Rob Herring]

On Mon, Jul 17, 2017 at 05:33:58PM +0530, Varadarajan Narayanan wrote:
> IPQ8074 uses QMP phy controller that provides support to PCIe and
> USB. Adding dt binding information for the same.
> 
> Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
> ---
>  .../devicetree/bindings/phy/qcom-qmp-phy.txt       | 28 ++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> index 5d7a51f..80d766b 100644
> --- a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
> @@ -6,6 +6,7 @@ controllers on Qualcomm chipsets, such as, PCIe, UFS, and USB.
>  
>  Required properties:
>   - compatible: compatible list, contains:
> +	       "qcom,ipq8074-qmp-pcie-phy" for PCIe phy on IPQ8074
>  	       "qcom,msm8996-qmp-pcie-phy" for 14nm PCIe phy on msm8996,
>  	       "qcom,msm8996-qmp-usb3-phy" for 14nm USB3 phy on msm8996.
>  
> @@ -107,3 +108,30 @@ Example:
>  		...
>  		...
>  	};
> +
> +	phy@86000 {

Only a new compatible doesn't warrant another example.

Rob

Re: [PATCH 6/7] dt-bindings: pci: qcom: Add support for IPQ8074

[Rob Herring]

On Mon, Jul 17, 2017 at 05:34:02PM +0530, Varadarajan Narayanan wrote:
> Add support for the IPQ8074 PCIe controller.  IPQ8074 supports Gen 1/2, one
> lane, two PCIe root complex with support for MSI and legacy interrupts, and
> it conforms to PCI Express Base 2.1 specification.
> 
> Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
> ---
>  .../devicetree/bindings/pci/qcom,pcie.txt          | 67 ++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> index 9d418b7..643bcc2 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> @@ -9,6 +9,7 @@
>  			- "qcom,pcie-apq8084" for apq8084
>  			- "qcom,pcie-msm8996" for msm8996 or apq8096
>  			- "qcom,pcie-ipq4019" for ipq4019
> +			- "qcom,pcie-ipq8074" for ipq8074
>  
>  - reg:
>  	Usage: required
> @@ -261,3 +262,69 @@
>  		pinctrl-0 = <&pcie0_pins_default>;
>  		pinctrl-names = "default";
>  	};
> +
> +* Example for ipq8074
> +	pcie0: pci@20000000 {

Same here. Just a new compatible doesn't need an example.

Rob

Re: [PATCH 7/7] PCI: dwc: qcom: Add support for IPQ8074 PCIe controller

[Bjorn Andersson]

On Mon 17 Jul 05:04 PDT 2017, Varadarajan Narayanan wrote:

> Add support for the IPQ8074 PCIe controller.  IPQ8074 supports
> Gen 1/2, one lane, two PCIe root complex with support for MSI and
> legacy interrupts, and it conforms to PCI Express Base 2.1
> specification.
> 
> The core init is the similar to the existing SoC, however the
> clocks and reset lines differ.
> 
> Signed-off-by: smuthayy <smuthayy@codeaurora.org>
> Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
> ---
>  drivers/pci/dwc/pcie-qcom.c | 259 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 259 insertions(+)
> 
> diff --git a/drivers/pci/dwc/pcie-qcom.c b/drivers/pci/dwc/pcie-qcom.c
> index d15657d..c1fa356 100644
> --- a/drivers/pci/dwc/pcie-qcom.c
> +++ b/drivers/pci/dwc/pcie-qcom.c
> @@ -37,6 +37,20 @@
>  #include "pcie-designware.h"
>  
>  #define PCIE20_PARF_SYS_CTRL			0x00
> +#define MST_WAKEUP_EN				BIT(13)
> +#define SLV_WAKEUP_EN				BIT(12)
> +#define MSTR_ACLK_CGC_DIS			BIT(10)
> +#define SLV_ACLK_CGC_DIS			BIT(9)
> +#define CORE_CLK_CGC_DIS			BIT(6)
> +#define AUX_PWR_DET				BIT(4)
> +#define L23_CLK_RMV_DIS				BIT(2)
> +#define L1_CLK_RMV_DIS				BIT(1)
> +
> +#define PCIE20_COMMAND_STATUS			0x04
> +#define CMD_BME_VAL				0x4
> +#define PCIE20_DEVICE_CONTROL2_STATUS2		0x98
> +#define PCIE_CAP_CPL_TIMEOUT_DISABLE		0x10
> +
>  #define PCIE20_PARF_PHY_CTRL			0x40
>  #define PCIE20_PARF_PHY_REFCLK			0x4C
>  #define PCIE20_PARF_DBI_BASE_ADDR		0x168
> @@ -58,9 +72,22 @@
>  #define CFG_BRIDGE_SB_INIT			BIT(0)
>  
>  #define PCIE20_CAP				0x70
> +#define PCIE20_CAP_LINK_CAPABILITIES		(PCIE20_CAP + 0xC)
> +#define PCIE20_CAP_LINK_1			(PCIE20_CAP + 0x14)
> +#define PCIE_CAP_LINK1_VAL			0x2fd7f
> +
> +#define PCIE20_PARF_Q2A_FLUSH			0x1AC
> +
> +#define PCIE20_MISC_CONTROL_1_REG		0x8BC
> +#define DBI_RO_WR_EN				1
>  
>  #define PERST_DELAY_US				1000
>  
> +#define AXI_CLK_RATE				200000000
> +
> +#define PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE	0x358
> +#define SLV_ADDR_SPACE_SZ                       0x10000000
> +
>  struct qcom_pcie_resources_v0 {
>  	struct clk *iface_clk;
>  	struct clk *core_clk;
> @@ -110,11 +137,26 @@ struct qcom_pcie_resources_v3 {
>  	struct reset_control *phy_ahb_reset;
>  };
>  
> +struct qphy_reset {
> +	struct reset_control	*rst;
> +	char			*name;
> +};
> +
> +struct qcom_pcie_resources_v4 {
> +	struct clk *sys_noc_clk;
> +	struct clk *axi_m_clk;
> +	struct clk *axi_s_clk;
> +	struct clk *ahb_clk;
> +	struct clk *aux_clk;
> +	struct qphy_reset rst[7];

Just store the struct reset_control here directly, carrying the name
doesn't serve much of a purpose - and it clutters the code.

> +};

Can you confirm that this is actually version 4 of this block? Or are we
just incrementing an arbitrary number here?

> +
>  union qcom_pcie_resources {
>  	struct qcom_pcie_resources_v0 v0;
>  	struct qcom_pcie_resources_v1 v1;
>  	struct qcom_pcie_resources_v2 v2;
>  	struct qcom_pcie_resources_v3 v3;
> +	struct qcom_pcie_resources_v4 v4;
>  };
>  
>  struct qcom_pcie;
> @@ -139,6 +181,16 @@ struct qcom_pcie {
>  
>  #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
>  
> +static inline void
> +writel_masked(void __iomem *addr, u32 clear_mask, u32 set_mask)

This function name is very generic and in the two places you call it
set_mask is 0. So please just inline this.

> +{
> +	u32 val = readl(addr);
> +
> +	val &= ~clear_mask;
> +	val |= set_mask;
> +	writel(val, addr);
> +}
> +
>  static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
>  {
>  	gpiod_set_value(pcie->reset, 1);
> @@ -884,6 +936,205 @@ static int qcom_pcie_init_v3(struct qcom_pcie *pcie)
>  	return ret;
>  }
>  
> +static int qcom_pcie_get_resources_v4(struct qcom_pcie *pcie)
> +{
> +	struct qcom_pcie_resources_v4 *res = &pcie->res.v4;
> +	struct dw_pcie *pci = pcie->pci;
> +	struct device *dev = pci->dev;
> +	int i;
> +
> +	res->sys_noc_clk = devm_clk_get(dev, "sys_noc");
> +	if (IS_ERR(res->sys_noc_clk))
> +		return PTR_ERR(res->sys_noc_clk);
> +
> +	res->axi_m_clk = devm_clk_get(dev, "axi_m");
> +	if (IS_ERR(res->axi_m_clk))
> +		return PTR_ERR(res->axi_m_clk);
> +
> +	res->axi_s_clk = devm_clk_get(dev, "axi_s");
> +	if (IS_ERR(res->axi_s_clk))
> +		return PTR_ERR(res->axi_s_clk);
> +
> +	res->ahb_clk = devm_clk_get(dev, "ahb");
> +	if (IS_ERR(res->ahb_clk))
> +		return PTR_ERR(res->ahb_clk);
> +
> +	res->aux_clk = devm_clk_get(dev, "aux");
> +	if (IS_ERR(res->aux_clk))
> +		return PTR_ERR(res->aux_clk);
> +
> +	res->rst[0].name = "axi_m";
> +	res->rst[1].name = "axi_s";
> +	res->rst[2].name = "pipe";
> +	res->rst[3].name = "axi_m_sticky";
> +	res->rst[4].name = "sticky";
> +	res->rst[5].name = "ahb";
> +	res->rst[6].name = "sleep";
> +
> +	for (i = 0; i < ARRAY_SIZE(res->rst); i++) {
> +		res->rst[i].rst = devm_reset_control_get(dev, res->rst[i].name);
> +		if (IS_ERR(res->rst[i].rst))
> +			return PTR_ERR(res->rst[i].rst);
> +	}
> +
> +	return 0;
> +}
> +
> +static void qcom_pcie_deinit_v4(struct qcom_pcie *pcie)
> +{
> +	struct qcom_pcie_resources_v4 *res = &pcie->res.v4;
> +
> +	clk_disable_unprepare(res->sys_noc_clk);
> +	clk_disable_unprepare(res->axi_m_clk);
> +	clk_disable_unprepare(res->axi_s_clk);
> +	clk_disable_unprepare(res->ahb_clk);
> +	clk_disable_unprepare(res->aux_clk);
> +}
> +
> +static int qcom_pcie_enable_resources_v4(struct qcom_pcie *pcie)
> +{
> +	struct qcom_pcie_resources_v4 *res = &pcie->res.v4;
> +	struct dw_pcie *pci = pcie->pci;
> +	struct device *dev = pci->dev;
> +	int ret;
> +
> +	ret = clk_prepare_enable(res->sys_noc_clk);
> +	if (ret) {
> +		dev_err(dev, "cannot prepare/enable core clock\n");
> +		return ret;
> +	}

Should these clocks really be handled explicitly in the driver? Are
these not the bus clocks, to be handled by "msm_bus"?

> +
> +	ret = clk_prepare_enable(res->axi_m_clk);
> +	if (ret) {
> +		dev_err(dev, "cannot prepare/enable core clock\n");
> +		goto err_clk_axi_m;
> +	}
> +
> +	ret = clk_set_rate(res->axi_m_clk, AXI_CLK_RATE);
> +	if (ret) {
> +		dev_err(dev, "MClk rate set failed (%d)\n", ret);
> +		goto err_clk_axi_m;
> +	}
> +
> +	ret = clk_prepare_enable(res->axi_s_clk);
> +	if (ret) {
> +		dev_err(dev, "cannot prepare/enable axi slave clock\n");
> +		goto err_clk_axi_s;
> +	}
> +
> +	ret = clk_set_rate(res->axi_s_clk, AXI_CLK_RATE);
> +	if (ret) {
> +		dev_err(dev, "MClk rate set failed (%d)\n", ret);
> +		goto err_clk_axi_s;
> +	}
> +
> +	ret = clk_prepare_enable(res->ahb_clk);
> +	if (ret) {
> +		dev_err(dev, "cannot prepare/enable ahb clock\n");
> +		goto err_clk_ahb;
> +	}
> +
> +	ret = clk_prepare_enable(res->aux_clk);
> +	if (ret) {
> +		dev_err(dev, "cannot prepare/enable aux clock\n");
> +		goto err_clk_aux;
> +	}
> +
> +	udelay(1);
> +
> +	return 0;
> +
> +err_clk_aux:
> +	clk_disable_unprepare(res->ahb_clk);
> +err_clk_ahb:
> +	clk_disable_unprepare(res->axi_s_clk);
> +err_clk_axi_s:
> +	clk_disable_unprepare(res->axi_m_clk);
> +err_clk_axi_m:
> +	clk_disable_unprepare(res->sys_noc_clk);
> +
> +	return ret;
> +}
> +
> +static inline int qphy_reset_control(struct qcom_pcie *pcie,
> +				     struct qphy_reset *r,
> +				     bool assert)
> +{
> +	int ret;
> +
> +	if (assert)
> +		ret = reset_control_assert(r->rst);
> +	else
> +		ret = reset_control_deassert(r->rst);
> +
> +	if (ret)
> +		dev_err(pcie->pci->dev, "%s: reset %s failed for %s\n",
> +			__func__, assert ? "assert" : "deassert", r->name);
> +
> +	return ret;
> +}
> +
> +static void qcom_pcie_v4_reset(struct qcom_pcie *pcie)
> +{
> +	struct qcom_pcie_resources_v4 *res = &pcie->res.v4;
> +	struct qphy_reset *qphy_rst = &res->rst[0];

&res->rst[0] is supposed to be written as res->rst, but that's exactly
the same number of characters to type as your local variable. So just
skip the variable.

> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(res->rst); i++)
> +		if (qphy_reset_control(pcie, &qphy_rst[i], true))

This is a complicated way of saying:
	if (reset_control_assert(qphy_rst[i].rst))

> +			return;

You should most definitely propagate this error.

> +
> +	usleep_range(10000, 12000); /* wait 12ms */

This is not 12ms, this is 10-12ms. This is a _long_ time to usleep, how
about just msleep(20) instead?

> +
> +	for (i = 0; i < ARRAY_SIZE(res->rst); i++)
> +		if (qphy_reset_control(pcie, &qphy_rst[i], false))

Same as above, this just write:
if (reset_control_deassert(qphy_rst[i].rst))

> +			return;
> +
> +	usleep_range(10000, 12000); /* wait 12ms */
> +	wmb(); /* ensure data is written to hw register */

wmb() ensures ordering of writes, it does not wait for data to reach the
hardware registers.

> +}
> +
> +static int qcom_pcie_init_v4(struct qcom_pcie *pcie)
> +{
> +	struct dw_pcie *pci = pcie->pci;
> +	int ret;
> +
> +	qcom_pcie_v4_reset(pcie);
> +	qcom_ep_reset_assert(pcie);
> +
> +	ret = qcom_pcie_enable_resources_v4(pcie);
> +	if (ret)
> +		return ret;
> +
> +	writel(SLV_ADDR_SPACE_SZ, pcie->parf +
> +					PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE);
> +
> +	ret = phy_power_on(pcie->phy);
> +	if (ret)

This will leave all the resources enabled, you should issue a deinit
here..

> +		return ret;
> +
> +	writel_masked(pcie->parf + PCIE20_PARF_PHY_CTRL, BIT(0), 0);
> +

Regards,
Bjorn

Re: [PATCH 3/7] phy: qcom-qmp: Fix phy pipe clock name

[Bjorn Andersson]

On Mon 17 Jul 05:03 PDT 2017, Varadarajan Narayanan wrote:

> Presently, the phy pipe clock's name is assumed to be either
> usb3_phy_pipe_clk_src or pcie_XX_pipe_clk_src (where XX is the
> phy lane's number). However, this will not work if an SoC has
> more than one instance of the phy. Hence, instead of assuming
> the name of the clock, fetch it from the DT.
> 

Adding the support to fetch this from DT looks reasonable, but you
should make sure to fall back to the old logic in case you don't find a
"clock-output-names" property.

[..]
> @@ -1110,6 +1103,8 @@ static int qcom_qmp_phy_probe(struct platform_device *pdev)
>  
>  	id = 0;
>  	for_each_available_child_of_node(dev->of_node, child) {
> +		const char *clk_name;
> +
>  		/* Create per-lane phy */
>  		ret = qcom_qmp_phy_create(dev, child, id);
>  		if (ret) {
> @@ -1118,11 +1113,20 @@ static int qcom_qmp_phy_probe(struct platform_device *pdev)
>  			return ret;
>  		}
>  
> +		ret = of_property_read_string(child, "clock-output-names",
> +							&clk_name);
> +		if (ret) {

This would be the case for any existing dts files, so you're not allowed
to treat this as an error.

> +			dev_err(dev,
> +				"failed to get clock-output-names for lane%d phy, %d\n",
> +				id, ret);
> +			return ret;
> +		}
> +

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/7] phy: qcom-qmp: Fix phy pipe clock name

[Bjorn Andersson]

On Mon 17 Jul 05:03 PDT 2017, Varadarajan Narayanan wrote:

> Presently, the phy pipe clock's name is assumed to be either
> usb3_phy_pipe_clk_src or pcie_XX_pipe_clk_src (where XX is the
> phy lane's number). However, this will not work if an SoC has
> more than one instance of the phy. Hence, instead of assuming
> the name of the clock, fetch it from the DT.
> 

Adding the support to fetch this from DT looks reasonable, but you
should make sure to fall back to the old logic in case you don't find a
"clock-output-names" property.

[..]
> @@ -1110,6 +1103,8 @@ static int qcom_qmp_phy_probe(struct platform_device *pdev)
>  
>  	id = 0;
>  	for_each_available_child_of_node(dev->of_node, child) {
> +		const char *clk_name;
> +
>  		/* Create per-lane phy */
>  		ret = qcom_qmp_phy_create(dev, child, id);
>  		if (ret) {
> @@ -1118,11 +1113,20 @@ static int qcom_qmp_phy_probe(struct platform_device *pdev)
>  			return ret;
>  		}
>  
> +		ret = of_property_read_string(child, "clock-output-names",
> +							&clk_name);
> +		if (ret) {

This would be the case for any existing dts files, so you're not allowed
to treat this as an error.

> +			dev_err(dev,
> +				"failed to get clock-output-names for lane%d phy, %d\n",
> +				id, ret);
> +			return ret;
> +		}
> +

Regards,
Bjorn

Re: [PATCH 3/7] phy: qcom-qmp: Fix phy pipe clock name

[Varadarajan Narayanan]

Bjorn,

On Mon, Jul 17, 2017 at 03:30:47PM -0700, Bjorn Andersson wrote:
> On Mon 17 Jul 05:03 PDT 2017, Varadarajan Narayanan wrote:
>
> > Presently, the phy pipe clock's name is assumed to be either
> > usb3_phy_pipe_clk_src or pcie_XX_pipe_clk_src (where XX is the
> > phy lane's number). However, this will not work if an SoC has
> > more than one instance of the phy. Hence, instead of assuming
> > the name of the clock, fetch it from the DT.
> >
>
> Adding the support to fetch this from DT looks reasonable, but you
> should make sure to fall back to the old logic in case you don't find a
> "clock-output-names" property.

If more than one instance of the IP block is instantiated in the
SoC, it will try to register the same clock (i.e.
pcie_0_pipe_clk_src or usb3_phy_pipe_clk_src) more than once and
devm_clk_hw_register() will fail for the second and subsequent
instances of the IP, resulting in the pipe_clk not being able to
find its parent.

Additionally, the clock name itself differs between SoCs. For
example, in MSM8996 it is usb3_phy_pipe_clk_src for USB and
pcie_X_pipe_clk_src (where X is lane number 0, 1 or 2). In
IPQ8074 it is usb3phy_X_cc_pipe_clk or pcie20_phyX_pipe_clk
(where X is IP instance number 0 or 1).

MSM8996, has one instance of the IP with 3 different lanes,
whereas IPQ8074 has two instances with one lane in each instance.
A future SoC might have multiple instances with each instance
having different number of lanes and the clock's name might have
X & Y to indicate instance number and lane numbers.

Hence, if the "clock-output-names" property is not available
there is no logic that can correctly "guess" the clock's name.
The old logic incorrectly assumes the format of the clock name,
resulting in devm_clk_hw_register() failures and pipe_clk not
being linked to its proper parent.

> [..]
> > @@ -1110,6 +1103,8 @@ static int qcom_qmp_phy_probe(struct platform_device *pdev)
> >
> >  	id = 0;
> >  	for_each_available_child_of_node(dev->of_node, child) {
> > +		const char *clk_name;
> > +
> >  		/* Create per-lane phy */
> >  		ret = qcom_qmp_phy_create(dev, child, id);
> >  		if (ret) {
> > @@ -1118,11 +1113,20 @@ static int qcom_qmp_phy_probe(struct platform_device *pdev)
> >  			return ret;
> >  		}
> >
> > +		ret = of_property_read_string(child, "clock-output-names",
> > +							&clk_name);
> > +		if (ret) {
>
> This would be the case for any existing dts files, so you're not allowed
> to treat this as an error.

Since, there are no dts files that presently enable this driver,
wouldn't it be better to flag this as an error now itself instead
of having to fall back to old handling (which as mentioned above
is incomplete). Please let me know.

Thanks
Varada

> > +			dev_err(dev,
> > +				"failed to get clock-output-names for lane%d phy, %d\n",
> > +				id, ret);
> > +			return ret;
> > +		}
> > +
>
> Regards,
> Bjorn

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 7/7] PCI: dwc: qcom: Add support for IPQ8074 PCIe controller

[Varadarajan Narayanan]

On Mon, Jul 17, 2017 at 03:07:18PM -0700, Bjorn Andersson wrote:
> On Mon 17 Jul 05:04 PDT 2017, Varadarajan Narayanan wrote:
>
> > Add support for the IPQ8074 PCIe controller.  IPQ8074 supports
> > Gen 1/2, one lane, two PCIe root complex with support for MSI and
> > legacy interrupts, and it conforms to PCI Express Base 2.1
> > specification.
> >
> > The core init is the similar to the existing SoC, however the
> > clocks and reset lines differ.
> >
> > Signed-off-by: smuthayy <smuthayy@codeaurora.org>
> > Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
> > ---
> >  drivers/pci/dwc/pcie-qcom.c | 259 ++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 259 insertions(+)
> >
> > diff --git a/drivers/pci/dwc/pcie-qcom.c b/drivers/pci/dwc/pcie-qcom.c
> > index d15657d..c1fa356 100644
> > --- a/drivers/pci/dwc/pcie-qcom.c
> > +++ b/drivers/pci/dwc/pcie-qcom.c
> > @@ -37,6 +37,20 @@
> >  #include "pcie-designware.h"
> >
> >  #define PCIE20_PARF_SYS_CTRL			0x00
> > +#define MST_WAKEUP_EN				BIT(13)
> > +#define SLV_WAKEUP_EN				BIT(12)
> > +#define MSTR_ACLK_CGC_DIS			BIT(10)
> > +#define SLV_ACLK_CGC_DIS			BIT(9)
> > +#define CORE_CLK_CGC_DIS			BIT(6)
> > +#define AUX_PWR_DET				BIT(4)
> > +#define L23_CLK_RMV_DIS				BIT(2)
> > +#define L1_CLK_RMV_DIS				BIT(1)
> > +
> > +#define PCIE20_COMMAND_STATUS			0x04
> > +#define CMD_BME_VAL				0x4
> > +#define PCIE20_DEVICE_CONTROL2_STATUS2		0x98
> > +#define PCIE_CAP_CPL_TIMEOUT_DISABLE		0x10
> > +
> >  #define PCIE20_PARF_PHY_CTRL			0x40
> >  #define PCIE20_PARF_PHY_REFCLK			0x4C
> >  #define PCIE20_PARF_DBI_BASE_ADDR		0x168
> > @@ -58,9 +72,22 @@
> >  #define CFG_BRIDGE_SB_INIT			BIT(0)
> >
> >  #define PCIE20_CAP				0x70
> > +#define PCIE20_CAP_LINK_CAPABILITIES		(PCIE20_CAP + 0xC)
> > +#define PCIE20_CAP_LINK_1			(PCIE20_CAP + 0x14)
> > +#define PCIE_CAP_LINK1_VAL			0x2fd7f
> > +
> > +#define PCIE20_PARF_Q2A_FLUSH			0x1AC
> > +
> > +#define PCIE20_MISC_CONTROL_1_REG		0x8BC
> > +#define DBI_RO_WR_EN				1
> >
> >  #define PERST_DELAY_US				1000
> >
> > +#define AXI_CLK_RATE				200000000
> > +
> > +#define PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE	0x358
> > +#define SLV_ADDR_SPACE_SZ                       0x10000000
> > +
> >  struct qcom_pcie_resources_v0 {
> >  	struct clk *iface_clk;
> >  	struct clk *core_clk;
> > @@ -110,11 +137,26 @@ struct qcom_pcie_resources_v3 {
> >  	struct reset_control *phy_ahb_reset;
> >  };
> >
> > +struct qphy_reset {
> > +	struct reset_control	*rst;
> > +	char			*name;
> > +};
> > +
> > +struct qcom_pcie_resources_v4 {
> > +	struct clk *sys_noc_clk;
> > +	struct clk *axi_m_clk;
> > +	struct clk *axi_s_clk;
> > +	struct clk *ahb_clk;
> > +	struct clk *aux_clk;
> > +	struct qphy_reset rst[7];
>
> Just store the struct reset_control here directly, carrying the name
> doesn't serve much of a purpose - and it clutters the code.

Ok.

> > +};
>
> Can you confirm that this is actually version 4 of this block? Or are we
> just incrementing an arbitrary number here?

This is not exactly the 4th version of the block. However, it is
a different version than the ones that are already supported in
this driver. Since the existing driver didn't exactly tie it with
the block IP version, I too followed the same versioning
convention.

> > +
> >  union qcom_pcie_resources {
> >  	struct qcom_pcie_resources_v0 v0;
> >  	struct qcom_pcie_resources_v1 v1;
> >  	struct qcom_pcie_resources_v2 v2;
> >  	struct qcom_pcie_resources_v3 v3;
> > +	struct qcom_pcie_resources_v4 v4;
> >  };
> >
> >  struct qcom_pcie;
> > @@ -139,6 +181,16 @@ struct qcom_pcie {
> >
> >  #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
> >
> > +static inline void
> > +writel_masked(void __iomem *addr, u32 clear_mask, u32 set_mask)
>
> This function name is very generic and in the two places you call it
> set_mask is 0. So please just inline this.

Ok.

> > +{
> > +	u32 val = readl(addr);
> > +
> > +	val &= ~clear_mask;
> > +	val |= set_mask;
> > +	writel(val, addr);
> > +}
> > +
> >  static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
> >  {
> >  	gpiod_set_value(pcie->reset, 1);
> > @@ -884,6 +936,205 @@ static int qcom_pcie_init_v3(struct qcom_pcie *pcie)
> >  	return ret;
> >  }
> >
> > +static int qcom_pcie_get_resources_v4(struct qcom_pcie *pcie)
> > +{
> > +	struct qcom_pcie_resources_v4 *res = &pcie->res.v4;
> > +	struct dw_pcie *pci = pcie->pci;
> > +	struct device *dev = pci->dev;
> > +	int i;
> > +
> > +	res->sys_noc_clk = devm_clk_get(dev, "sys_noc");
> > +	if (IS_ERR(res->sys_noc_clk))
> > +		return PTR_ERR(res->sys_noc_clk);
> > +
> > +	res->axi_m_clk = devm_clk_get(dev, "axi_m");
> > +	if (IS_ERR(res->axi_m_clk))
> > +		return PTR_ERR(res->axi_m_clk);
> > +
> > +	res->axi_s_clk = devm_clk_get(dev, "axi_s");
> > +	if (IS_ERR(res->axi_s_clk))
> > +		return PTR_ERR(res->axi_s_clk);
> > +
> > +	res->ahb_clk = devm_clk_get(dev, "ahb");
> > +	if (IS_ERR(res->ahb_clk))
> > +		return PTR_ERR(res->ahb_clk);
> > +
> > +	res->aux_clk = devm_clk_get(dev, "aux");
> > +	if (IS_ERR(res->aux_clk))
> > +		return PTR_ERR(res->aux_clk);
> > +
> > +	res->rst[0].name = "axi_m";
> > +	res->rst[1].name = "axi_s";
> > +	res->rst[2].name = "pipe";
> > +	res->rst[3].name = "axi_m_sticky";
> > +	res->rst[4].name = "sticky";
> > +	res->rst[5].name = "ahb";
> > +	res->rst[6].name = "sleep";
> > +
> > +	for (i = 0; i < ARRAY_SIZE(res->rst); i++) {
> > +		res->rst[i].rst = devm_reset_control_get(dev, res->rst[i].name);
> > +		if (IS_ERR(res->rst[i].rst))
> > +			return PTR_ERR(res->rst[i].rst);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static void qcom_pcie_deinit_v4(struct qcom_pcie *pcie)
> > +{
> > +	struct qcom_pcie_resources_v4 *res = &pcie->res.v4;
> > +
> > +	clk_disable_unprepare(res->sys_noc_clk);
> > +	clk_disable_unprepare(res->axi_m_clk);
> > +	clk_disable_unprepare(res->axi_s_clk);
> > +	clk_disable_unprepare(res->ahb_clk);
> > +	clk_disable_unprepare(res->aux_clk);
> > +}
> > +
> > +static int qcom_pcie_enable_resources_v4(struct qcom_pcie *pcie)
> > +{
> > +	struct qcom_pcie_resources_v4 *res = &pcie->res.v4;
> > +	struct dw_pcie *pci = pcie->pci;
> > +	struct device *dev = pci->dev;
> > +	int ret;
> > +
> > +	ret = clk_prepare_enable(res->sys_noc_clk);
> > +	if (ret) {
> > +		dev_err(dev, "cannot prepare/enable core clock\n");
> > +		return ret;
> > +	}
>
> Should these clocks really be handled explicitly in the driver? Are
> these not the bus clocks, to be handled by "msm_bus"?

This not the bus clock. This clock is for the Bus Interface Unit
between the PCIe module and the System NOC.

> > +
> > +	ret = clk_prepare_enable(res->axi_m_clk);
> > +	if (ret) {
> > +		dev_err(dev, "cannot prepare/enable core clock\n");
> > +		goto err_clk_axi_m;
> > +	}
> > +
> > +	ret = clk_set_rate(res->axi_m_clk, AXI_CLK_RATE);
> > +	if (ret) {
> > +		dev_err(dev, "MClk rate set failed (%d)\n", ret);
> > +		goto err_clk_axi_m;
> > +	}
> > +
> > +	ret = clk_prepare_enable(res->axi_s_clk);
> > +	if (ret) {
> > +		dev_err(dev, "cannot prepare/enable axi slave clock\n");
> > +		goto err_clk_axi_s;
> > +	}
> > +
> > +	ret = clk_set_rate(res->axi_s_clk, AXI_CLK_RATE);
> > +	if (ret) {
> > +		dev_err(dev, "MClk rate set failed (%d)\n", ret);
> > +		goto err_clk_axi_s;
> > +	}
> > +
> > +	ret = clk_prepare_enable(res->ahb_clk);
> > +	if (ret) {
> > +		dev_err(dev, "cannot prepare/enable ahb clock\n");
> > +		goto err_clk_ahb;
> > +	}
> > +
> > +	ret = clk_prepare_enable(res->aux_clk);
> > +	if (ret) {
> > +		dev_err(dev, "cannot prepare/enable aux clock\n");
> > +		goto err_clk_aux;
> > +	}
> > +
> > +	udelay(1);
> > +
> > +	return 0;
> > +
> > +err_clk_aux:
> > +	clk_disable_unprepare(res->ahb_clk);
> > +err_clk_ahb:
> > +	clk_disable_unprepare(res->axi_s_clk);
> > +err_clk_axi_s:
> > +	clk_disable_unprepare(res->axi_m_clk);
> > +err_clk_axi_m:
> > +	clk_disable_unprepare(res->sys_noc_clk);
> > +
> > +	return ret;
> > +}
> > +
> > +static inline int qphy_reset_control(struct qcom_pcie *pcie,
> > +				     struct qphy_reset *r,
> > +				     bool assert)
> > +{
> > +	int ret;
> > +
> > +	if (assert)
> > +		ret = reset_control_assert(r->rst);
> > +	else
> > +		ret = reset_control_deassert(r->rst);
> > +
> > +	if (ret)
> > +		dev_err(pcie->pci->dev, "%s: reset %s failed for %s\n",
> > +			__func__, assert ? "assert" : "deassert", r->name);
> > +
> > +	return ret;
> > +}
> > +
> > +static void qcom_pcie_v4_reset(struct qcom_pcie *pcie)
> > +{
> > +	struct qcom_pcie_resources_v4 *res = &pcie->res.v4;
> > +	struct qphy_reset *qphy_rst = &res->rst[0];
>
> &res->rst[0] is supposed to be written as res->rst, but that's exactly
> the same number of characters to type as your local variable. So just
> skip the variable.

Ok.

> > +	int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(res->rst); i++)
> > +		if (qphy_reset_control(pcie, &qphy_rst[i], true))
>
> This is a complicated way of saying:
> 	if (reset_control_assert(qphy_rst[i].rst))

Ok, will remove the helper function.

> > +			return;
>
> You should most definitely propagate this error.

Ok.

> > +
> > +	usleep_range(10000, 12000); /* wait 12ms */
>
> This is not 12ms, this is 10-12ms. This is a _long_ time to usleep, how
> about just msleep(20) instead?

Ok.

> > +
> > +	for (i = 0; i < ARRAY_SIZE(res->rst); i++)
> > +		if (qphy_reset_control(pcie, &qphy_rst[i], false))
>
> Same as above, this just write:
> if (reset_control_deassert(qphy_rst[i].rst))

Ok.

> > +			return;
> > +
> > +	usleep_range(10000, 12000); /* wait 12ms */
> > +	wmb(); /* ensure data is written to hw register */
>
> wmb() ensures ordering of writes, it does not wait for data to reach the
> hardware registers.

Ok. Will remove it.

> > +}
> > +
> > +static int qcom_pcie_init_v4(struct qcom_pcie *pcie)
> > +{
> > +	struct dw_pcie *pci = pcie->pci;
> > +	int ret;
> > +
> > +	qcom_pcie_v4_reset(pcie);
> > +	qcom_ep_reset_assert(pcie);
> > +
> > +	ret = qcom_pcie_enable_resources_v4(pcie);
> > +	if (ret)
> > +		return ret;
> > +
> > +	writel(SLV_ADDR_SPACE_SZ, pcie->parf +
> > +					PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE);
> > +
> > +	ret = phy_power_on(pcie->phy);
> > +	if (ret)
>
> This will leave all the resources enabled, you should issue a deinit
> here..

Ok.

Thanks
Varada

> > +		return ret;
> > +
> > +	writel_masked(pcie->parf + PCIE20_PARF_PHY_CTRL, BIT(0), 0);
> > +
>
> Regards,
> Bjorn

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 7/7] PCI: dwc: qcom: Add support for IPQ8074 PCIe controller

[Bjorn Andersson]

On Tue 18 Jul 02:58 PDT 2017, Varadarajan Narayanan wrote:

> On Mon, Jul 17, 2017 at 03:07:18PM -0700, Bjorn Andersson wrote:
> > On Mon 17 Jul 05:04 PDT 2017, Varadarajan Narayanan wrote:
[..]
> >
> > Can you confirm that this is actually version 4 of this block? Or are we
> > just incrementing an arbitrary number here?
> 
> This is not exactly the 4th version of the block. However, it is
> a different version than the ones that are already supported in
> this driver. Since the existing driver didn't exactly tie it with
> the block IP version, I too followed the same versioning
> convention.
> 

Do you have a block IP version that you could base your numbering on, to
break the trend? (We should go back and fix up the others as well)

[..]
> > > +static int qcom_pcie_enable_resources_v4(struct qcom_pcie *pcie)
> > > +{
> > > +	struct qcom_pcie_resources_v4 *res = &pcie->res.v4;
> > > +	struct dw_pcie *pci = pcie->pci;
> > > +	struct device *dev = pci->dev;
> > > +	int ret;
> > > +
> > > +	ret = clk_prepare_enable(res->sys_noc_clk);
> > > +	if (ret) {
> > > +		dev_err(dev, "cannot prepare/enable core clock\n");
> > > +		return ret;
> > > +	}
> >
> > Should these clocks really be handled explicitly in the driver? Are
> > these not the bus clocks, to be handled by "msm_bus"?
> 
> This not the bus clock. This clock is for the Bus Interface Unit
> between the PCIe module and the System NOC.
> 

Right, that was the piece I meant. Sorry for not using the right
nomenclature.

So then it would be handled by the msm_bus in the downstream kernel?


Perhaps we can merge it like this and once we have the interconnect
framework setup we can make this the fallback method.

Thanks,
Bjorn

Re: [PATCH 3/7] phy: qcom-qmp: Fix phy pipe clock name

[Bjorn Andersson]

On Tue 18 Jul 01:54 PDT 2017, Varadarajan Narayanan wrote:
> On Mon, Jul 17, 2017 at 03:30:47PM -0700, Bjorn Andersson wrote:
[..]
> >
> > This would be the case for any existing dts files, so you're not allowed
> > to treat this as an error.
> 
> Since, there are no dts files that presently enable this driver,
> wouldn't it be better to flag this as an error now itself instead
> of having to fall back to old handling (which as mentioned above
> is incomplete). Please let me know.
> 

You're right, hopefully we can introduce it in the upstream dts for
v4.14...

So it does make sense to fix this up now, rather than live with the
fallback forever. Can you ensure we get an "ack" from Vivek on this?

Regards,
Bjorn

Re: [PATCH 3/7] phy: qcom-qmp: Fix phy pipe clock name

[Vivek Gautam]

On Tue, Jul 18, 2017 at 10:26 PM, Bjorn Andersson
<bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Tue 18 Jul 01:54 PDT 2017, Varadarajan Narayanan wrote:
>> On Mon, Jul 17, 2017 at 03:30:47PM -0700, Bjorn Andersson wrote:
> [..]
>> >
>> > This would be the case for any existing dts files, so you're not allowed
>> > to treat this as an error.
>>
>> Since, there are no dts files that presently enable this driver,
>> wouldn't it be better to flag this as an error now itself instead
>> of having to fall back to old handling (which as mentioned above
>> is incomplete). Please let me know.
>>
>
> You're right, hopefully we can introduce it in the upstream dts for
> v4.14...
>
> So it does make sense to fix this up now, rather than live with the
> fallback forever. Can you ensure we get an "ack" from Vivek on this?

Right, it makes sense to get this name from the dt (since same
will be also available in the clock entry), and not assume the name
based on some logic.
And since we don't have any already existing bindings, it's good that
we fix it now.

>
> Regards,
> Bjorn
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/7] phy: qcom-qmp: Fix phy pipe clock name

[Vivek Gautam]

On Tue, Jul 18, 2017 at 10:26 PM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> On Tue 18 Jul 01:54 PDT 2017, Varadarajan Narayanan wrote:
>> On Mon, Jul 17, 2017 at 03:30:47PM -0700, Bjorn Andersson wrote:
> [..]
>> >
>> > This would be the case for any existing dts files, so you're not allowed
>> > to treat this as an error.
>>
>> Since, there are no dts files that presently enable this driver,
>> wouldn't it be better to flag this as an error now itself instead
>> of having to fall back to old handling (which as mentioned above
>> is incomplete). Please let me know.
>>
>
> You're right, hopefully we can introduce it in the upstream dts for
> v4.14...
>
> So it does make sense to fix this up now, rather than live with the
> fallback forever. Can you ensure we get an "ack" from Vivek on this?

Right, it makes sense to get this name from the dt (since same
will be also available in the clock entry), and not assume the name
based on some logic.
And since we don't have any already existing bindings, it's good that
we fix it now.

>
> Regards,
> Bjorn
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 3/7] phy: qcom-qmp: Fix phy pipe clock name

[Vivek Gautam]

Hi,


On Mon, Jul 17, 2017 at 5:33 PM, Varadarajan Narayanan
<varada-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> Presently, the phy pipe clock's name is assumed to be either
> usb3_phy_pipe_clk_src or pcie_XX_pipe_clk_src (where XX is the
> phy lane's number). However, this will not work if an SoC has
> more than one instance of the phy. Hence, instead of assuming
> the name of the clock, fetch it from the DT.
>
> Signed-off-by: Varadarajan Narayanan <varada-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---

Looks good.

Acked-by: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>


>  drivers/phy/qualcomm/phy-qcom-qmp.c | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c
> index 78ca628..97020ec 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
> @@ -925,20 +925,13 @@ static int qcom_qmp_phy_clk_init(struct device *dev)
>   *    clk  |   +-------+   |                   +-----+
>   *         +---------------+
>   */
> -static int phy_pipe_clk_register(struct qcom_qmp *qmp, int id)
> +static int phy_pipe_clk_register(struct qcom_qmp *qmp, const char *clk_name)
>  {
> -       char name[24];
>         struct clk_fixed_rate *fixed;
>         struct clk_init_data init = { };
>
> -       switch (qmp->cfg->type) {
> -       case PHY_TYPE_USB3:
> -               snprintf(name, sizeof(name), "usb3_phy_pipe_clk_src");
> -               break;
> -       case PHY_TYPE_PCIE:
> -               snprintf(name, sizeof(name), "pcie_%d_pipe_clk_src", id);
> -               break;
> -       default:
> +       if ((qmp->cfg->type != PHY_TYPE_USB3) &&
> +           (qmp->cfg->type != PHY_TYPE_PCIE)) {
>                 /* not all phys register pipe clocks, so return success */
>                 return 0;
>         }
> @@ -947,7 +940,7 @@ static int phy_pipe_clk_register(struct qcom_qmp *qmp, int id)
>         if (!fixed)
>                 return -ENOMEM;
>
> -       init.name = name;
> +       init.name = clk_name;
>         init.ops = &clk_fixed_rate_ops;
>
>         /* controllers using QMP phys use 125MHz pipe clock interface */
> @@ -1110,6 +1103,8 @@ static int qcom_qmp_phy_probe(struct platform_device *pdev)
>
>         id = 0;
>         for_each_available_child_of_node(dev->of_node, child) {
> +               const char *clk_name;
> +
>                 /* Create per-lane phy */
>                 ret = qcom_qmp_phy_create(dev, child, id);
>                 if (ret) {
> @@ -1118,11 +1113,20 @@ static int qcom_qmp_phy_probe(struct platform_device *pdev)
>                         return ret;
>                 }
>
> +               ret = of_property_read_string(child, "clock-output-names",
> +                                                       &clk_name);
> +               if (ret) {
> +                       dev_err(dev,
> +                               "failed to get clock-output-names for lane%d phy, %d\n",
> +                               id, ret);
> +                       return ret;
> +               }
> +
>                 /*
>                  * Register the pipe clock provided by phy.
>                  * See function description to see details of this pipe clock.
>                  */
> -               ret = phy_pipe_clk_register(qmp, id);
> +               ret = phy_pipe_clk_register(qmp, clk_name);
>                 if (ret) {
>                         dev_err(qmp->dev,
>                                 "failed to register pipe clock source\n");
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/7] phy: qcom-qmp: Fix phy pipe clock name

[Vivek Gautam]

Hi,


On Mon, Jul 17, 2017 at 5:33 PM, Varadarajan Narayanan
<varada@codeaurora.org> wrote:
> Presently, the phy pipe clock's name is assumed to be either
> usb3_phy_pipe_clk_src or pcie_XX_pipe_clk_src (where XX is the
> phy lane's number). However, this will not work if an SoC has
> more than one instance of the phy. Hence, instead of assuming
> the name of the clock, fetch it from the DT.
>
> Signed-off-by: Varadarajan Narayanan <varada@codeaurora.org>
> ---

Looks good.

Acked-by: Vivek Gautam <vivek.gautam@codeaurora.org>


>  drivers/phy/qualcomm/phy-qcom-qmp.c | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c
> index 78ca628..97020ec 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
> @@ -925,20 +925,13 @@ static int qcom_qmp_phy_clk_init(struct device *dev)
>   *    clk  |   +-------+   |                   +-----+
>   *         +---------------+
>   */
> -static int phy_pipe_clk_register(struct qcom_qmp *qmp, int id)
> +static int phy_pipe_clk_register(struct qcom_qmp *qmp, const char *clk_name)
>  {
> -       char name[24];
>         struct clk_fixed_rate *fixed;
>         struct clk_init_data init = { };
>
> -       switch (qmp->cfg->type) {
> -       case PHY_TYPE_USB3:
> -               snprintf(name, sizeof(name), "usb3_phy_pipe_clk_src");
> -               break;
> -       case PHY_TYPE_PCIE:
> -               snprintf(name, sizeof(name), "pcie_%d_pipe_clk_src", id);
> -               break;
> -       default:
> +       if ((qmp->cfg->type != PHY_TYPE_USB3) &&
> +           (qmp->cfg->type != PHY_TYPE_PCIE)) {
>                 /* not all phys register pipe clocks, so return success */
>                 return 0;
>         }
> @@ -947,7 +940,7 @@ static int phy_pipe_clk_register(struct qcom_qmp *qmp, int id)
>         if (!fixed)
>                 return -ENOMEM;
>
> -       init.name = name;
> +       init.name = clk_name;
>         init.ops = &clk_fixed_rate_ops;
>
>         /* controllers using QMP phys use 125MHz pipe clock interface */
> @@ -1110,6 +1103,8 @@ static int qcom_qmp_phy_probe(struct platform_device *pdev)
>
>         id = 0;
>         for_each_available_child_of_node(dev->of_node, child) {
> +               const char *clk_name;
> +
>                 /* Create per-lane phy */
>                 ret = qcom_qmp_phy_create(dev, child, id);
>                 if (ret) {
> @@ -1118,11 +1113,20 @@ static int qcom_qmp_phy_probe(struct platform_device *pdev)
>                         return ret;
>                 }
>
> +               ret = of_property_read_string(child, "clock-output-names",
> +                                                       &clk_name);
> +               if (ret) {
> +                       dev_err(dev,
> +                               "failed to get clock-output-names for lane%d phy, %d\n",
> +                               id, ret);
> +                       return ret;
> +               }
> +
>                 /*
>                  * Register the pipe clock provided by phy.
>                  * See function description to see details of this pipe clock.
>                  */
> -               ret = phy_pipe_clk_register(qmp, id);
> +               ret = phy_pipe_clk_register(qmp, clk_name);
>                 if (ret) {
>                         dev_err(qmp->dev,
>                                 "failed to register pipe clock source\n");
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 7/7] PCI: dwc: qcom: Add support for IPQ8074 PCIe controller

[Varadarajan Narayanan]

On Tue, Jul 18, 2017 at 09:44:38AM -0700, Bjorn Andersson wrote:
> On Tue 18 Jul 02:58 PDT 2017, Varadarajan Narayanan wrote:
>
> > On Mon, Jul 17, 2017 at 03:07:18PM -0700, Bjorn Andersson wrote:
> > > On Mon 17 Jul 05:04 PDT 2017, Varadarajan Narayanan wrote:
> [..]
> > >
> > > Can you confirm that this is actually version 4 of this block? Or are we
> > > just incrementing an arbitrary number here?
> >
> > This is not exactly the 4th version of the block. However, it is
> > a different version than the ones that are already supported in
> > this driver. Since the existing driver didn't exactly tie it with
> > the block IP version, I too followed the same versioning
> > convention.
> >
>
> Do you have a block IP version that you could base your numbering on, to
> break the trend? (We should go back and fix up the others as well)

Presently, the driver supports the ipq8064, apq8064, apq8084,
msm8996, ipq4019 and ipq8074. The SoCs, qcom_pcie_ops version and
the block IP versions are as follows.

	ipq8064 - v0 - 2.1.0
	apq8064 - v0 - 2.1.0
	apq8084 - v1 - 1.0.0
	msm8996 - v2 - 2.3.2
	ipq4019 - v3 - 2.4.0
	ipq8074 - v4 - 2.3.3

I will rename the qcom_pcie_ops structure and related functions
with the block IP version instead of vX numbering and post the
patch.

> [..]
> > > > +static int qcom_pcie_enable_resources_v4(struct qcom_pcie *pcie)
> > > > +{
> > > > +	struct qcom_pcie_resources_v4 *res = &pcie->res.v4;
> > > > +	struct dw_pcie *pci = pcie->pci;
> > > > +	struct device *dev = pci->dev;
> > > > +	int ret;
> > > > +
> > > > +	ret = clk_prepare_enable(res->sys_noc_clk);
> > > > +	if (ret) {
> > > > +		dev_err(dev, "cannot prepare/enable core clock\n");
> > > > +		return ret;
> > > > +	}
> > >
> > > Should these clocks really be handled explicitly in the driver? Are
> > > these not the bus clocks, to be handled by "msm_bus"?
> >
> > This not the bus clock. This clock is for the Bus Interface Unit
> > between the PCIe module and the System NOC.
> >
>
> Right, that was the piece I meant. Sorry for not using the right
> nomenclature.
>
> So then it would be handled by the msm_bus in the downstream kernel?
>
>
> Perhaps we can merge it like this and once we have the interconnect
> framework setup we can make this the fallback method.

Agree that this has to be handled by the interconnect framework.
For now will rename this as "iface" similar to v0 and v1.

Thanks
Varada

> Thanks,
> Bjorn

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/7] PCI: dwc: qcom: Add support for IPQ8074 PCIe controller

[Varadarajan Narayanan]

On Tue, Jul 18, 2017 at 09:44:38AM -0700, Bjorn Andersson wrote:
> On Tue 18 Jul 02:58 PDT 2017, Varadarajan Narayanan wrote:
>
> > On Mon, Jul 17, 2017 at 03:07:18PM -0700, Bjorn Andersson wrote:
> > > On Mon 17 Jul 05:04 PDT 2017, Varadarajan Narayanan wrote:
> [..]
> > >
> > > Can you confirm that this is actually version 4 of this block? Or are we
> > > just incrementing an arbitrary number here?
> >
> > This is not exactly the 4th version of the block. However, it is
> > a different version than the ones that are already supported in
> > this driver. Since the existing driver didn't exactly tie it with
> > the block IP version, I too followed the same versioning
> > convention.
> >
>
> Do you have a block IP version that you could base your numbering on, to
> break the trend? (We should go back and fix up the others as well)

Presently, the driver supports the ipq8064, apq8064, apq8084,
msm8996, ipq4019 and ipq8074. The SoCs, qcom_pcie_ops version and
the block IP versions are as follows.

	ipq8064 - v0 - 2.1.0
	apq8064 - v0 - 2.1.0
	apq8084 - v1 - 1.0.0
	msm8996 - v2 - 2.3.2
	ipq4019 - v3 - 2.4.0
	ipq8074 - v4 - 2.3.3

I will rename the qcom_pcie_ops structure and related functions
with the block IP version instead of vX numbering and post the
patch.

> [..]
> > > > +static int qcom_pcie_enable_resources_v4(struct qcom_pcie *pcie)
> > > > +{
> > > > +	struct qcom_pcie_resources_v4 *res = &pcie->res.v4;
> > > > +	struct dw_pcie *pci = pcie->pci;
> > > > +	struct device *dev = pci->dev;
> > > > +	int ret;
> > > > +
> > > > +	ret = clk_prepare_enable(res->sys_noc_clk);
> > > > +	if (ret) {
> > > > +		dev_err(dev, "cannot prepare/enable core clock\n");
> > > > +		return ret;
> > > > +	}
> > >
> > > Should these clocks really be handled explicitly in the driver? Are
> > > these not the bus clocks, to be handled by "msm_bus"?
> >
> > This not the bus clock. This clock is for the Bus Interface Unit
> > between the PCIe module and the System NOC.
> >
>
> Right, that was the piece I meant. Sorry for not using the right
> nomenclature.
>
> So then it would be handled by the msm_bus in the downstream kernel?
>
>
> Perhaps we can merge it like this and once we have the interconnect
> framework setup we can make this the fallback method.

Agree that this has to be handled by the interconnect framework.
For now will rename this as "iface" similar to v0 and v1.

Thanks
Varada

> Thanks,
> Bjorn

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 7/7] PCI: dwc: qcom: Add support for IPQ8074 PCIe controller

[Stanimir Varbanov]

Hi,

On 07/19/2017 09:49 AM, Varadarajan Narayanan wrote:
> On Tue, Jul 18, 2017 at 09:44:38AM -0700, Bjorn Andersson wrote:
>> On Tue 18 Jul 02:58 PDT 2017, Varadarajan Narayanan wrote:
>>
>>> On Mon, Jul 17, 2017 at 03:07:18PM -0700, Bjorn Andersson wrote:
>>>> On Mon 17 Jul 05:04 PDT 2017, Varadarajan Narayanan wrote:
>> [..]
>>>>
>>>> Can you confirm that this is actually version 4 of this block? Or are we
>>>> just incrementing an arbitrary number here?
>>>
>>> This is not exactly the 4th version of the block. However, it is
>>> a different version than the ones that are already supported in
>>> this driver. Since the existing driver didn't exactly tie it with
>>> the block IP version, I too followed the same versioning
>>> convention.
>>>
>>
>> Do you have a block IP version that you could base your numbering on, to
>> break the trend? (We should go back and fix up the others as well)
> 
> Presently, the driver supports the ipq8064, apq8064, apq8084,
> msm8996, ipq4019 and ipq8074. The SoCs, qcom_pcie_ops version and
> the block IP versions are as follows.
> 
> 	ipq8064 - v0 - 2.1.0
> 	apq8064 - v0 - 2.1.0
> 	apq8084 - v1 - 1.0.0
> 	msm8996 - v2 - 2.3.2
> 	ipq4019 - v3 - 2.4.0
> 	ipq8074 - v4 - 2.3.3

That's nice, but I think we need the Synopsys IP versions too, can you
provide such an information and after that we can decide how the names
should look like.

> 
> I will rename the qcom_pcie_ops structure and related functions
> with the block IP version instead of vX numbering and post the
> patch.



regards,
Stan

Re: [PATCH 7/7] PCI: dwc: qcom: Add support for IPQ8074 PCIe controller

[Varadarajan Narayanan]

Stan,

On Wed, Jul 19, 2017 at 10:12:45AM +0300, Stanimir Varbanov wrote:
> Hi,
>
> On 07/19/2017 09:49 AM, Varadarajan Narayanan wrote:
> > On Tue, Jul 18, 2017 at 09:44:38AM -0700, Bjorn Andersson wrote:
> >> On Tue 18 Jul 02:58 PDT 2017, Varadarajan Narayanan wrote:
> >>
> >>> On Mon, Jul 17, 2017 at 03:07:18PM -0700, Bjorn Andersson wrote:
> >>>> On Mon 17 Jul 05:04 PDT 2017, Varadarajan Narayanan wrote:
> >> [..]
> >>>>
> >>>> Can you confirm that this is actually version 4 of this block? Or are we
> >>>> just incrementing an arbitrary number here?
> >>>
> >>> This is not exactly the 4th version of the block. However, it is
> >>> a different version than the ones that are already supported in
> >>> this driver. Since the existing driver didn't exactly tie it with
> >>> the block IP version, I too followed the same versioning
> >>> convention.
> >>>
> >>
> >> Do you have a block IP version that you could base your numbering on, to
> >> break the trend? (We should go back and fix up the others as well)
> >
> > Presently, the driver supports the ipq8064, apq8064, apq8084,
> > msm8996, ipq4019 and ipq8074. The SoCs, qcom_pcie_ops version and
> > the block IP versions are as follows.
> >
> > 	ipq8064 - v0 - 2.1.0
> > 	apq8064 - v0 - 2.1.0
> > 	apq8084 - v1 - 1.0.0
> > 	msm8996 - v2 - 2.3.2
> > 	ipq4019 - v3 - 2.4.0
> > 	ipq8074 - v4 - 2.3.3
>
> That's nice, but I think we need the Synopsys IP versions too, can you
> provide such an information and after that we can decide how the names
> should look like.

Sorry, I posted v2 before I saw this e-mail. Will post v3 based
on what naming style is decided.

The SoCs, qcom_pcie_ops version, the block IP version and
Synopsys IP versions are as follows.

	ipq8064 - v0 - 2.1.0 - 4.01a
	apq8064 - v0 - 2.1.0 - 4.01a
	apq8084 - v1 - 1.0.0 - 4.11a
	msm8996 - v2 - 2.3.2 - 4.21a
	ipq4019 - v3 - 2.4.0 - 4.20a
	ipq8074 - v4 - 2.3.3 - 4.30a

Thanks
Varada

> > I will rename the qcom_pcie_ops structure and related functions
> > with the block IP version instead of vX numbering and post the
> > patch.
>
>
>
> regards,
> Stan

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 7/7] PCI: dwc: qcom: Add support for IPQ8074 PCIe controller

[Vivek Gautam]

Hi,


On 07/19/2017 02:59 PM, Varadarajan Narayanan wrote:
> Stan,
>
> On Wed, Jul 19, 2017 at 10:12:45AM +0300, Stanimir Varbanov wrote:
>> Hi,
>>
>> On 07/19/2017 09:49 AM, Varadarajan Narayanan wrote:
>>> On Tue, Jul 18, 2017 at 09:44:38AM -0700, Bjorn Andersson wrote:
>>>> On Tue 18 Jul 02:58 PDT 2017, Varadarajan Narayanan wrote:
>>>>
>>>>> On Mon, Jul 17, 2017 at 03:07:18PM -0700, Bjorn Andersson wrote:
>>>>>> On Mon 17 Jul 05:04 PDT 2017, Varadarajan Narayanan wrote:
>>>> [..]
>>>>>> Can you confirm that this is actually version 4 of this block? Or are we
>>>>>> just incrementing an arbitrary number here?
>>>>> This is not exactly the 4th version of the block. However, it is
>>>>> a different version than the ones that are already supported in
>>>>> this driver. Since the existing driver didn't exactly tie it with
>>>>> the block IP version, I too followed the same versioning
>>>>> convention.
>>>>>
>>>> Do you have a block IP version that you could base your numbering on, to
>>>> break the trend? (We should go back and fix up the others as well)
>>> Presently, the driver supports the ipq8064, apq8064, apq8084,
>>> msm8996, ipq4019 and ipq8074. The SoCs, qcom_pcie_ops version and
>>> the block IP versions are as follows.
>>>
>>> 	ipq8064 - v0 - 2.1.0
>>> 	apq8064 - v0 - 2.1.0
>>> 	apq8084 - v1 - 1.0.0
>>> 	msm8996 - v2 - 2.3.2
>>> 	ipq4019 - v3 - 2.4.0
>>> 	ipq8074 - v4 - 2.3.3
>> That's nice, but I think we need the Synopsys IP versions too, can you
>> provide such an information and after that we can decide how the names
>> should look like.
> Sorry, I posted v2 before I saw this e-mail. Will post v3 based
> on what naming style is decided.
>
> The SoCs, qcom_pcie_ops version, the block IP version and
> Synopsys IP versions are as follows.
>
> 	ipq8064 - v0 - 2.1.0 - 4.01a
> 	apq8064 - v0 - 2.1.0 - 4.01a
> 	apq8084 - v1 - 1.0.0 - 4.11a
> 	msm8996 - v2 - 2.3.2 - 4.21a
> 	ipq4019 - v3 - 2.4.0 - 4.20a
> 	ipq8074 - v4 - 2.3.3 - 4.30a

I would have loved to say the dwc IP versions sounds better, but
this is the qcom wrapper over dwc. So, for me it makes more sense
to have qcom specific IP version names.
Can we have couple or more versions of qcom pcie IPs based on
same dwc IP version? I have not looked closely, but in that case
too using qcom nomenclature would again make more sense.


Thanks
Vivek

>
> Thanks
> Varada
>
>>> I will rename the qcom_pcie_ops structure and related functions
>>> with the block IP version instead of vX numbering and post the
>>> patch.
>>
>>
>> regards,
>> Stan
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 7/7] PCI: dwc: qcom: Add support for IPQ8074 PCIe controller

[Stanimir Varbanov]

Hi,

On 07/19/2017 12:29 PM, Varadarajan Narayanan wrote:
> Stan,
> 

<cut>

>> That's nice, but I think we need the Synopsys IP versions too, can you
>> provide such an information and after that we can decide how the names
>> should look like.
> 
> Sorry, I posted v2 before I saw this e-mail. Will post v3 based
> on what naming style is decided.
> 
> The SoCs, qcom_pcie_ops version, the block IP version and
> Synopsys IP versions are as follows.
> 
> 	ipq8064 - v0 - 2.1.0 - 4.01a
> 	apq8064 - v0 - 2.1.0 - 4.01a
> 	apq8084 - v1 - 1.0.0 - 4.11a
> 	msm8996 - v2 - 2.3.2 - 4.21a
> 	ipq4019 - v3 - 2.4.0 - 4.20a
> 	ipq8074 - v4 - 2.3.3 - 4.30a

I'm not sure, with Synopsys versions the mess becomes bigger.

So I tend to agree with qcom versions (because this driver taking care
of qcom wrappers over Synopsys IP) plus comments about Synopsys version
used for this particular SoC, just for completeness.


regards,
Stan