linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] Qualcomm PCIe and PCIe/PHY drivers
@ 2015-05-04 12:42 Stanimir Varbanov
  2015-05-04 12:42 ` [PATCH v2 1/5] DT: phy: qcom: Add PCIe PHY devicetree bindings Stanimir Varbanov
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Stanimir Varbanov @ 2015-05-04 12:42 UTC (permalink / raw)
  To: Rob Herring, Kumar Gala, Mark Rutland, Grant Likely,
	Bjorn Helgaas, Kishon Vijay Abraham I, Russell King,
	Arnd Bergmann
  Cc: linux-arm-msm, linux-kernel, linux-arm-kernel, devicetree,
	linux-pci, Mathieu Olivari, Srinivas Kandagatla,
	Stanimir Varbanov

From: Stanimir Varbanov <stanimir.varbanov@linaro.org>

Hi,

Here is a v2 of the patchset. The chages since v1 are:

 - take into accoint the review comments from the inital version
 - add support for pcie IP found on Snapdragon 600 (ipq/apq8064)

Version 1 of the patchset can be found at: 

https://lkml.org/lkml/2014/12/12/557

Commenst are welcome!

-------------------------------------------------------------------------------

This patchset introduces a Qualcomm PCIe root complex driver for
Snapdragon 805 (APQ8084). It also adds a PCIe PHY driver in generic
phy framework. The PCIe hardware use Designware IP core plus
Qualcomm application specific hw.

The first two patches add a PHY driver binding document and the 
PHY driver. Patches 3/5 and 4/5 add PCIe DT document and the driver.
Last 5/5 adds APQ8084 in mach-qcom as next multiplatform Qualcomm
SoC.

Stanimir Varbanov (5):
  DT: phy: qcom: Add PCIe PHY devicetree bindings
  phy: qcom: Add Qualcomm PCIe PHY
  DT: PCI: qcom: Document PCIe devicetree bindings
  PCI: qcom: Add Qualcomm PCIe controller driver
  ARM: qcom: Add Qualcomm APQ8084 SoC

 .../devicetree/bindings/pci/qcom,pcie.txt          |  231 +++++++
 .../devicetree/bindings/phy/qcom-pcie-phy.txt      |   60 ++
 MAINTAINERS                                        |    7 +
 arch/arm/mach-qcom/Kconfig                         |    7 +
 drivers/pci/host/Kconfig                           |    9 +
 drivers/pci/host/Makefile                          |    1 +
 drivers/pci/host/pcie-qcom.c                       |  677 ++++++++++++++++++++
 drivers/phy/Kconfig                                |    9 +
 drivers/phy/Makefile                               |    1 +
 drivers/phy/phy-qcom-pcie.c                        |  291 +++++++++
 10 files changed, 1293 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/qcom,pcie.txt
 create mode 100644 Documentation/devicetree/bindings/phy/qcom-pcie-phy.txt
 create mode 100644 drivers/pci/host/pcie-qcom.c
 create mode 100644 drivers/phy/phy-qcom-pcie.c


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2 1/5] DT: phy: qcom: Add PCIe PHY devicetree bindings
  2015-05-04 12:42 [PATCH v2 0/5] Qualcomm PCIe and PCIe/PHY drivers Stanimir Varbanov
@ 2015-05-04 12:42 ` Stanimir Varbanov
  2015-05-04 12:42 ` [PATCH v2 2/5] phy: qcom: Add Qualcomm PCIe PHY Stanimir Varbanov
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 17+ messages in thread
From: Stanimir Varbanov @ 2015-05-04 12:42 UTC (permalink / raw)
  To: Rob Herring, Kumar Gala, Mark Rutland, Grant Likely,
	Bjorn Helgaas, Kishon Vijay Abraham I, Russell King,
	Arnd Bergmann
  Cc: linux-arm-msm, linux-kernel, linux-arm-kernel, devicetree,
	linux-pci, Mathieu Olivari, Srinivas Kandagatla,
	Stanimir Varbanov

Document Qualcomm PCIe PHY devicetree bindings.

Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
---
 .../devicetree/bindings/phy/qcom-pcie-phy.txt      |   60 ++++++++++++++++++++
 1 files changed, 60 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/qcom-pcie-phy.txt

diff --git a/Documentation/devicetree/bindings/phy/qcom-pcie-phy.txt b/Documentation/devicetree/bindings/phy/qcom-pcie-phy.txt
new file mode 100644
index 0000000..e4c7f9c
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom-pcie-phy.txt
@@ -0,0 +1,60 @@
+* Qualcomm PCIe PHY controller
+
+PCIe PHY nodes are defined to describe on-chip PCIe Physical layer controllers.
+Each PCIe PHY controller should have its own node.
+
+- compatible:
+	Usage: required
+	Value type: <stringlist>
+	Definition: Value should contain "qcom,pcie-phy"
+
+- reg:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: Offset and length of the PCIe PHY registers
+
+- #phy-cells:
+	Usage: required
+	Value type: <u32>
+	Definition: Must be zero
+
+- clocks:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: A list of phandles and clock specifier pair, one
+		    for each entry in clock-names property
+
+- clock-names:
+	Usage: required
+	Value type: <stringlist>
+	Definition: Should contain "core" for PHY core clock
+
+- resets:
+	Usage: required
+	Value type: <phandle>
+	Definition: List of phandle and reset specifier pairs as listed
+		    in reset-names property
+
+- reset-names:
+	Usage: required
+	Value type: <stringlist>
+	Definition: Should contain "core" for PHY core reset
+
+- <name>-supply:
+	Usage: required
+	Value type: <phandle>
+	Definition: List of phandles to the power supply regulators
+		    - "vdda_pll" analog Vdd PLL power supply
+
+* Example
+
+	pciephy0: phy@fc526000 {
+		compatible = "qcom,pcie-phy";
+		reg = <0xfc526000 0x1000>;
+		clocks = <&gcc GCC_PCIE_0_PIPE_CLK>;
+		clock-names = "core";
+		resets = <&gcc GCC_PCIE_0_PHY_BCR>;
+		reset-names = "core";
+		vdda_pll-supply = <&pma8084_l12>;
+		#phy-cells = <0>;
+	};
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 2/5] phy: qcom: Add Qualcomm PCIe PHY
  2015-05-04 12:42 [PATCH v2 0/5] Qualcomm PCIe and PCIe/PHY drivers Stanimir Varbanov
  2015-05-04 12:42 ` [PATCH v2 1/5] DT: phy: qcom: Add PCIe PHY devicetree bindings Stanimir Varbanov
@ 2015-05-04 12:42 ` Stanimir Varbanov
  2015-05-04 14:35   ` Kishon Vijay Abraham I
  2015-05-04 12:42 ` [PATCH v2 3/5] DT: PCI: qcom: Document PCIe devicetree bindings Stanimir Varbanov
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Stanimir Varbanov @ 2015-05-04 12:42 UTC (permalink / raw)
  To: Rob Herring, Kumar Gala, Mark Rutland, Grant Likely,
	Bjorn Helgaas, Kishon Vijay Abraham I, Russell King,
	Arnd Bergmann
  Cc: linux-arm-msm, linux-kernel, linux-arm-kernel, devicetree,
	linux-pci, Mathieu Olivari, Srinivas Kandagatla,
	Stanimir Varbanov

Add a PCIe PHY driver used by PCIe host controller driver
on Qualcomm SoCs like Snapdragon 805.

Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
---
 drivers/phy/Kconfig         |    9 ++
 drivers/phy/Makefile        |    1 +
 drivers/phy/phy-qcom-pcie.c |  291 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 301 insertions(+), 0 deletions(-)
 create mode 100644 drivers/phy/phy-qcom-pcie.c

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index a53bd5b..6cf9065 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -257,6 +257,15 @@ config PHY_QCOM_IPQ806X_SATA
 	depends on OF
 	select GENERIC_PHY
 
+config PHY_QCOM_PCIE
+	tristate "Qualcomm PCIe SerDes/PHY driver"
+	depends on ARCH_QCOM
+	depends on HAS_IOMEM
+	depends on OF
+	select GENERIC_PHY
+	help
+	  Enable support for PCIe phy driver
+
 config PHY_ROCKCHIP_USB
 	tristate "Rockchip USB2 PHY Driver"
 	depends on ARCH_ROCKCHIP && OF
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index f126251..d80b99b 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_PHY_EXYNOS5_USBDRD)	+= phy-exynos5-usbdrd.o
 obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)	+= phy-qcom-apq8064-sata.o
 obj-$(CONFIG_PHY_ROCKCHIP_USB) += phy-rockchip-usb.o
 obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA)	+= phy-qcom-ipq806x-sata.o
+obj-$(CONFIG_PHY_QCOM_PCIE)		+= phy-qcom-pcie.o
 obj-$(CONFIG_PHY_ST_SPEAR1310_MIPHY)	+= phy-spear1310-miphy.o
 obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY)	+= phy-spear1340-miphy.o
 obj-$(CONFIG_PHY_XGENE)			+= phy-xgene.o
diff --git a/drivers/phy/phy-qcom-pcie.c b/drivers/phy/phy-qcom-pcie.c
new file mode 100644
index 0000000..0ce2fc6
--- /dev/null
+++ b/drivers/phy/phy-qcom-pcie.c
@@ -0,0 +1,291 @@
+/*
+ * Copyright (c) 2014, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+
+#define QSERDES_COM_PLL_CP_SETI			0x024
+#define QSERDES_COM_PLL_IP_SETP			0x028
+#define QSERDES_COM_PLL_CP_SETP			0x02c
+#define QSERDES_COM_SYSCLK_EN_SEL		0x038
+#define QSERDES_COM_RESETSM_CNTRL		0x040
+#define QSERDES_COM_PLLLOCK_CMP1		0x044
+#define QSERDES_COM_PLLLOCK_CMP2		0x048
+#define QSERDES_COM_PLLLOCK_CMP_EN		0x050
+#define QSERDES_COM_DEC_START1			0x064
+#define QSERDES_COM_DIV_FRAC_START1		0x098
+#define QSERDES_COM_DIV_FRAC_START2		0x09c
+#define QSERDES_COM_DIV_FRAC_START3		0x0a0
+#define QSERDES_COM_DEC_START2			0x0a4
+#define QSERDES_COM_PLL_RXTXEPCLK_EN		0x0a8
+#define QSERDES_COM_PLL_CRCTRL			0x0ac
+
+#define QSERDES_RX_CDR_CONTROL			0x400
+#define QSERDES_RX_CDR_CONTROL2			0x410
+#define QSERDES_RX_RX_TERM_HIGHZ_CM_AC_COUPLE	0x42c
+#define QSERDES_RX_RX_EQ_GAIN12			0x430
+
+#define PCIE_PHY_SW_RESET			0x600
+#define PCIE_PHY_POWER_DOWN_CONTROL		0x604
+#define PCIE_PHY_START				0x608
+#define PCIE_PHY_ENDPOINT_REFCLK_DRIVE		0x648
+#define PCIE_PHY_POWER_STATE_CONFIG1		0x650
+#define PCIE_PHY_POWER_STATE_CONFIG2		0x654
+#define PCIE_PHY_PWRUP_RESET_DLY_TIME_SYSCLK	0x678
+#define PCIE_PHY_PWRUP_RESET_DLY_TIME_AUXCLK	0x67c
+#define PCIE_PHY_PCS_STATUS			0x6c8
+
+#define PHY_DELAY_MIN_US			995
+#define PHY_DELAY_MAX_US			1005
+#define PHY_RETRIES_COUNT			10
+
+#define PIPE_CLK_DELAY_MIN_US			5000
+#define PIPE_CLK_DELAY_MAX_US			5100
+#define PIPE_CLK_RETRIES_COUNT			10
+
+struct qcom_pcie_phy {
+	void __iomem *base;
+	struct clk *clk;
+	struct reset_control *res_phy;
+	struct regulator *vdda_pll;
+	struct device *dev;
+};
+
+struct phy_regs {
+	u32 reg_offset;
+	u32 val;
+};
+
+static const struct phy_regs pcie_phy_pwrup[] = {
+	{ PCIE_PHY_POWER_DOWN_CONTROL,			0x03 },
+	{ QSERDES_COM_SYSCLK_EN_SEL,			0x08 },
+	{ QSERDES_COM_DEC_START1,			0x82 },
+	{ QSERDES_COM_DEC_START2,			0x03 },
+	{ QSERDES_COM_DIV_FRAC_START1,			0xd5 },
+	{ QSERDES_COM_DIV_FRAC_START2,			0xaa },
+	{ QSERDES_COM_DIV_FRAC_START3,			0x13 },
+	{ QSERDES_COM_PLLLOCK_CMP_EN,			0x01 },
+	{ QSERDES_COM_PLLLOCK_CMP1,			0x2b },
+	{ QSERDES_COM_PLLLOCK_CMP2,			0x68 },
+	{ QSERDES_COM_PLL_CRCTRL,			0xff },
+	{ QSERDES_COM_PLL_CP_SETI,			0x3f },
+	{ QSERDES_COM_PLL_IP_SETP,			0x07 },
+	{ QSERDES_COM_PLL_CP_SETP,			0x03 },
+	{ QSERDES_RX_CDR_CONTROL,			0xf3 },
+	{ QSERDES_RX_CDR_CONTROL2,			0x6b },
+	{ QSERDES_COM_RESETSM_CNTRL,			0x10 },
+	{ QSERDES_RX_RX_TERM_HIGHZ_CM_AC_COUPLE,	0x87 },
+	{ QSERDES_RX_RX_EQ_GAIN12,			0x54 },
+	{ PCIE_PHY_POWER_STATE_CONFIG1,			0xa3 },
+	{ PCIE_PHY_POWER_STATE_CONFIG2,			0xcb },
+	{ QSERDES_COM_PLL_RXTXEPCLK_EN,			0x10 },
+	{ PCIE_PHY_ENDPOINT_REFCLK_DRIVE,		0x10 },
+	{ PCIE_PHY_SW_RESET,				0x00 },
+	{ PCIE_PHY_START,				0x03 },
+};
+
+static const struct phy_regs pcie_phy_pwrdown[] = {
+	{ PCIE_PHY_SW_RESET,				0x01 },
+	{ PCIE_PHY_POWER_DOWN_CONTROL,			0x00 },
+};
+
+static void qcom_pcie_phy_pwr(struct qcom_pcie_phy *pcie,
+			      const struct phy_regs *regs, int arr_size)
+{
+	int i;
+
+	for (i = 0; i < arr_size; i++)
+		writel(regs[i].val, pcie->base + regs[i].reg_offset);
+}
+
+static void qcom_pcie_phy_pwrup(struct qcom_pcie_phy *pcie)
+{
+	qcom_pcie_phy_pwr(pcie, pcie_phy_pwrup, ARRAY_SIZE(pcie_phy_pwrup));
+}
+
+static void qcom_pcie_phy_pwrdown(struct qcom_pcie_phy *pcie)
+{
+	qcom_pcie_phy_pwr(pcie, pcie_phy_pwrdown, ARRAY_SIZE(pcie_phy_pwrdown));
+}
+
+static bool qcom_pcie_phy_is_ready(struct qcom_pcie_phy *pcie)
+{
+	u32 val = readl(pcie->base + PCIE_PHY_PCS_STATUS);
+
+	return val & BIT(6) ? false : true;
+}
+
+static int qcom_pcie_phy_power_on(struct phy *phy)
+{
+	struct qcom_pcie_phy *pcie = phy_get_drvdata(phy);
+	struct device *dev = pcie->dev;
+	int ret, retries;
+
+	ret = regulator_enable(pcie->vdda_pll);
+	if (ret) {
+		dev_err(dev, "cannot enable vdda_pll regulator\n");
+		return ret;
+	}
+
+	ret = reset_control_deassert(pcie->res_phy);
+	if (ret) {
+		dev_err(dev, "cannot deassert phy reset\n");
+		goto fail_vdda_pll;
+	}
+
+	qcom_pcie_phy_pwrup(pcie);
+
+	ret = clk_set_rate(pcie->clk, ~0);
+	if (ret) {
+		dev_err(dev, "cannot set phy clock rate\n");
+		goto fail_res;
+	}
+
+	/*
+	 * setting pipe rate takes time, try arbitrary delay before enabling
+	 * the clock
+	 */
+	retries = PIPE_CLK_RETRIES_COUNT;
+	do {
+		usleep_range(PIPE_CLK_DELAY_MIN_US, PIPE_CLK_DELAY_MAX_US);
+
+		ret = clk_prepare_enable(pcie->clk);
+		if (!ret)
+			break;
+	} while (retries--);
+
+	if (retries < 0) {
+		dev_err(dev, "cannot enable phy clock\n");
+		goto fail_res;
+	}
+
+	retries = PHY_RETRIES_COUNT;
+	do {
+		ret = qcom_pcie_phy_is_ready(pcie);
+		if (ret)
+			break;
+		usleep_range(PHY_DELAY_MIN_US, PHY_DELAY_MAX_US);
+	} while (retries--);
+
+	if (retries < 0) {
+		dev_err(dev, "cannot power on phy\n");
+		ret = -ETIMEDOUT;
+		goto fail;
+	}
+
+	return 0;
+
+fail:
+	clk_disable_unprepare(pcie->clk);
+fail_res:
+	reset_control_assert(pcie->res_phy);
+fail_vdda_pll:
+	regulator_disable(pcie->vdda_pll);
+
+	return ret;
+}
+
+static int qcom_pcie_phy_power_off(struct phy *phy)
+{
+	struct qcom_pcie_phy *pcie = phy_get_drvdata(phy);
+
+	clk_disable_unprepare(pcie->clk);
+	qcom_pcie_phy_pwrdown(pcie);
+	reset_control_assert(pcie->res_phy);
+	regulator_disable(pcie->vdda_pll);
+
+	return 0;
+}
+
+static const struct phy_ops qcom_pcie_phy_ops = {
+	.power_on = qcom_pcie_phy_power_on,
+	.power_off = qcom_pcie_phy_power_off,
+	.owner = THIS_MODULE,
+};
+
+static int qcom_pcie_phy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct phy_provider *provider;
+	struct qcom_pcie_phy *pcie;
+	struct resource *res;
+	struct phy *phy;
+
+	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
+	if (!pcie)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	pcie->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(pcie->base))
+		return PTR_ERR(pcie->base);
+
+	pcie->clk = devm_clk_get(dev, "core");
+	if (IS_ERR(pcie->clk))
+		return PTR_ERR(pcie->clk);
+
+	pcie->vdda_pll = devm_regulator_get(dev, "vdda_pll");
+	if (IS_ERR(pcie->vdda_pll))
+		return PTR_ERR(pcie->vdda_pll);
+
+	pcie->res_phy = devm_reset_control_get(dev, "core");
+	if (IS_ERR(pcie->res_phy))
+		return PTR_ERR(pcie->res_phy);
+
+	phy = devm_phy_create(dev, NULL, &qcom_pcie_phy_ops);
+	if (IS_ERR(phy)) {
+		dev_err(dev, "cannot create phy\n");
+		return PTR_ERR(phy);
+	}
+
+	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	if (IS_ERR(provider)) {
+		dev_err(dev, "cannot register phy provider\n");
+		return PTR_ERR(provider);
+	}
+
+	pcie->dev = dev;
+	phy_set_drvdata(phy, pcie);
+	platform_set_drvdata(pdev, pcie);
+
+	return 0;
+}
+
+static const struct of_device_id qcom_pcie_phy_of_match[] = {
+	{ .compatible = "qcom,pcie-phy" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, qcom_pcie_phy_of_match);
+
+static struct platform_driver qcom_pcie_phy_driver = {
+	.probe = qcom_pcie_phy_probe,
+	.driver = {
+		.name = "pcie-phy",
+		.of_match_table = qcom_pcie_phy_of_match,
+	}
+};
+
+module_platform_driver(qcom_pcie_phy_driver);
+
+MODULE_AUTHOR("Stanimir Varbanov <svarbanov@mm-sol.com>");
+MODULE_DESCRIPTION("Qualcomm PCIe PHY driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:pcie-phy");
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 3/5] DT: PCI: qcom: Document PCIe devicetree bindings
  2015-05-04 12:42 [PATCH v2 0/5] Qualcomm PCIe and PCIe/PHY drivers Stanimir Varbanov
  2015-05-04 12:42 ` [PATCH v2 1/5] DT: phy: qcom: Add PCIe PHY devicetree bindings Stanimir Varbanov
  2015-05-04 12:42 ` [PATCH v2 2/5] phy: qcom: Add Qualcomm PCIe PHY Stanimir Varbanov
@ 2015-05-04 12:42 ` Stanimir Varbanov
  2015-05-04 12:42 ` [PATCH v2 4/5] PCI: qcom: Add Qualcomm PCIe controller driver Stanimir Varbanov
  2015-05-04 12:42 ` [PATCH v2 5/5] ARM: qcom: Add Qualcomm APQ8084 SoC Stanimir Varbanov
  4 siblings, 0 replies; 17+ messages in thread
From: Stanimir Varbanov @ 2015-05-04 12:42 UTC (permalink / raw)
  To: Rob Herring, Kumar Gala, Mark Rutland, Grant Likely,
	Bjorn Helgaas, Kishon Vijay Abraham I, Russell King,
	Arnd Bergmann
  Cc: linux-arm-msm, linux-kernel, linux-arm-kernel, devicetree,
	linux-pci, Mathieu Olivari, Srinivas Kandagatla,
	Stanimir Varbanov

Document Qualcomm PCIe driver devicetree bindings.

Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
---
 .../devicetree/bindings/pci/qcom,pcie.txt          |  231 ++++++++++++++++++++
 1 files changed, 231 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/qcom,pcie.txt

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
new file mode 100644
index 0000000..dcf7348
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
@@ -0,0 +1,231 @@
+* Qualcomm PCI express root complex
+
+- compatible:
+	Usage: required
+	Value type: <stringlist>
+	Definition: Value shall include
+		    - "qcom,pcie-v0" for apq/ipq8064
+		    - "qcom,pcie-v1" for apq8084
+
+- reg:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: Register ranges as listed in the reg-names property
+
+- reg-names:
+	Usage: required
+	Value type: <stringlist>
+	Definition: Must include the following entries
+		    - "parf"   Qualcomm specific registers
+		    - "dbi"    Designware PCIe registers
+		    - "elbi"   External local bus interface registers
+		    - "config" PCIe configuration space
+
+- device_type:
+	Usage: required
+	Value type: <string>
+	Definition: Should be "pci". As specified in designware-pcie.txt
+
+- #address-cells:
+	Usage: required
+	Value type: <u32>
+	Definition: Should be set to 3. As specified in designware-pcie.txt
+
+- #size-cells:
+	Usage: required
+	Value type: <u32>
+	Definition: Should be set 2. As specified in designware-pcie.txt
+
+- ranges:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: As specified in designware-pcie.txt
+
+- interrupts:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: MSI interrupt
+
+- interrupt-names:
+	Usage: required
+	Value type: <stringlist>
+	Definition: Should contain "msi"
+
+- #interrupt-cells:
+	Usage: required
+	Value type: <u32>
+	Definition: Should be 1. As specified in designware-pcie.txt
+
+- interrupt-map-mask:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: As specified in designware-pcie.txt
+
+- interrupt-map:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: As specified in designware-pcie.txt
+
+- clocks:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: List of phandle and clock specifier pairs as listed
+		    in clock-names property
+
+- clock-names:
+	Usage: required
+	Value type: <stringlist>
+	Definition: Should contain the following entries
+		    * should be populated for v0 and v1
+		    	- "iface"      Configuration AHB clock
+
+		    * should be populated for v0
+		    	- "core"       Clocks the pcie hw block
+		    	- "phy"        Clocks the pcie PHY block
+
+		    * should be populated for v1
+			- "aux"        Auxiliary (AUX) clock
+		    	- "bus_master" Master AXI clock
+		    	- "bus_slave"  Slave AXI clock
+
+- resets:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: List of phandle and reset specifier pairs as listed
+		    in reset-names property
+
+- reset-names:
+	Usage: required
+	Value type: <stringlist>
+	Definition: Should contain the following entries
+		    * should be populated for v0
+			- "axi"  AXI reset
+			- "ahb"  AHB reset
+			- "por"  POR reset
+			- "pci"  PCI reset
+			- "phy"  PHY reset
+
+		    * should be populated for v1
+			- "core" Core reset
+
+- power-domains:
+	Usage: required (for v1 only)
+	Value type: <prop-encoded-array>
+	Definition: A phandle and power domain specifier pair to the
+		    power domain which is responsible for collapsing
+		    and restoring power to the peripheral
+
+- <name>-supply:
+	Usage: required
+	Value type: <phandle>
+	Definition: List of phandles to the power supply regulator(s)
+		    * should be populated for v0 and v1
+			- "vdda"        core analog power supply
+
+		    * should be populated for v0
+			- "vdda_phy"    analog power supply for PHY
+			- "vdda_refclk" analog power supply for IC which generate
+					reference clock
+
+- phys:
+	Usage: required (for v1 only)
+	Value type: <phandle>
+	Definition: List of phandle(s) as listed in phy-names property
+
+- phy-names:
+	Usage: required (for v1 only)
+	Value type: <stringlist>
+	Definition: Should contain "pciephy"
+
+- <name>-gpio:
+	Usage: optional
+	Value type: <prop-encoded-array>
+	Definition: List of phandle and gpio specifier pairs. Should contain
+		    - "perst"  PCIe endpoint reset signal line
+		    - "pewake" PCIe endpoint wake signal line
+
+- pinctrl-0:
+	Usage: required
+	Value type: <phandle>
+	Definition: List of phandles pointing at a pin(s) configuration
+
+- pinctrl-names
+	Usage: required
+	Value type: <stringlist>
+	Definition: List of names of pinctrl-0 state
+
+* Example for v0
+	pcie0: pci@1b500000 {
+		compatible = "qcom,pcie-v0";
+		reg = <0x1b500000 0x1000
+		       0x1b502000 0x80
+		       0x1b600000 0x100
+		       0x0ff00000 0x100000>;
+		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>;
+		ranges = <0x81000000 0 0	  0x0fe00000 0 0x00100000   /* I/O */
+			  0x82000000 0 0x00000000 0x08000000 0 0x07e00000>; /* memory */
+		interrupts = <GIC_SPI 35 IRQ_TYPE_NONE>;
+		interrupt-names = "msi";
+		#interrupt-cells = <1>;
+		interrupt-map-mask = <0 0 0 0x7>;
+		interrupt-map = <0 0 0 1 &intc 0 36 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
+				<0 0 0 2 &intc 0 37 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
+				<0 0 0 3 &intc 0 38 IRQ_TYPE_LEVEL_HIGH>, /* int_c */
+				<0 0 0 4 &intc 0 39 IRQ_TYPE_LEVEL_HIGH>; /* int_d */
+		clocks = <&gcc PCIE_A_CLK>,
+			 <&gcc PCIE_H_CLK>,
+			 <&gcc PCIE_PHY_CLK>;
+		clock-names = "core", "iface", "phy";
+		resets = <&gcc PCIE_ACLK_RESET>,
+			 <&gcc PCIE_HCLK_RESET>,
+			 <&gcc PCIE_POR_RESET>,
+			 <&gcc PCIE_PCI_RESET>,
+			 <&gcc PCIE_PHY_RESET>;
+		reset-names = "axi", "ahb", "por", "pci", "phy";
+	};
+
+* Example for v1
+	pcie0@fc520000 {
+		compatible = "qcom,pcie-v1";
+		reg = <0xfc520000 0x2000>,
+		      <0xff000000 0x1000>,
+		      <0xff001000 0x1000>,
+		      <0xff002000 0x2000>;
+		reg-names = "parf", "dbi", "elbi", "config";
+		device_type = "pci";
+		linux,pci-domain = <0>;
+		bus-range = <0x00 0xff>;
+		num-lanes = <1>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		ranges = <0x81000000 0 0          0xff200000 0 0x00100000   /* I/O */
+			  0x82000000 0 0x00300000 0xff300000 0 0x00d00000>; /* memory */
+		interrupts = <GIC_SPI 243 IRQ_TYPE_NONE>;
+		interrupt-names = "msi";
+		#interrupt-cells = <1>;
+		interrupt-map-mask = <0 0 0 0x7>;
+		interrupt-map = <0 0 0 1 &intc 0 244 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
+				<0 0 0 2 &intc 0 245 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
+				<0 0 0 3 &intc 0 247 IRQ_TYPE_LEVEL_HIGH>, /* int_c */
+				<0 0 0 4 &intc 0 248 IRQ_TYPE_LEVEL_HIGH>; /* int_d */
+		clocks = <&gcc GCC_PCIE_0_CFG_AHB_CLK>,
+			 <&gcc GCC_PCIE_0_MSTR_AXI_CLK>,
+			 <&gcc GCC_PCIE_0_SLV_AXI_CLK>,
+			 <&gcc GCC_PCIE_0_AUX_CLK>;
+		clock-names = "iface", "master_bus", "slave_bus", "aux";
+		resets = <&gcc GCC_PCIE_0_BCR>;
+		reset-names = "core";
+		power-domains = <&gcc PCIE0_GDSC>;
+		vdda-supply = <&pma8084_l3>;
+		phys = <&pciephy0>;
+		phy-names = "pciephy";
+		perst-gpio = <&tlmm 70 GPIO_ACTIVE_LOW>;
+		pinctrl-0 = <&pcie0_pins_default>;
+		pinctrl-names = "default";
+	};
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 4/5] PCI: qcom: Add Qualcomm PCIe controller driver
  2015-05-04 12:42 [PATCH v2 0/5] Qualcomm PCIe and PCIe/PHY drivers Stanimir Varbanov
                   ` (2 preceding siblings ...)
  2015-05-04 12:42 ` [PATCH v2 3/5] DT: PCI: qcom: Document PCIe devicetree bindings Stanimir Varbanov
@ 2015-05-04 12:42 ` Stanimir Varbanov
  2015-11-06 20:50   ` Bjorn Andersson
  2015-05-04 12:42 ` [PATCH v2 5/5] ARM: qcom: Add Qualcomm APQ8084 SoC Stanimir Varbanov
  4 siblings, 1 reply; 17+ messages in thread
From: Stanimir Varbanov @ 2015-05-04 12:42 UTC (permalink / raw)
  To: Rob Herring, Kumar Gala, Mark Rutland, Grant Likely,
	Bjorn Helgaas, Kishon Vijay Abraham I, Russell King,
	Arnd Bergmann
  Cc: linux-arm-msm, linux-kernel, linux-arm-kernel, devicetree,
	linux-pci, Mathieu Olivari, Srinivas Kandagatla,
	Stanimir Varbanov

The PCIe driver reuse the Designware common code for host
and MSI initialization, and also program the Qualcomm
application specific registers.

Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
---
 MAINTAINERS                  |    7 +
 drivers/pci/host/Kconfig     |    9 +
 drivers/pci/host/Makefile    |    1 +
 drivers/pci/host/pcie-qcom.c |  677 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 694 insertions(+), 0 deletions(-)
 create mode 100644 drivers/pci/host/pcie-qcom.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 781e099..1ce2628 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7564,6 +7564,13 @@ L:	linux-pci@vger.kernel.org
 S:	Orphan
 F:	drivers/pci/host/*spear*
 
+PCIE DRIVER FOR QUALCOMM MSM
+M:	Stanimir Varbanov <svarbanov@mm-sol.com>
+L:	linux-pci@vger.kernel.org
+L:	linux-arm-msm@vger.kernel.org
+S:	Maintained
+F:	drivers/pci/host/*qcom*
+
 PCMCIA SUBSYSTEM
 P:	Linux PCMCIA Team
 L:	linux-pcmcia@lists.infradead.org
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 1dfb567..1dc2757 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -125,4 +125,13 @@ config PCIE_IPROC_PLATFORM
 	  Say Y here if you want to use the Broadcom iProc PCIe controller
 	  through the generic platform bus interface
 
+config PCIE_QCOM
+	bool "Qualcomm PCIe controller"
+	depends on ARCH_QCOM && OF || (ARM && COMPILE_TEST)
+	select PCIE_DW
+	select PCIEPORTBUS
+	help
+	  Say Y here to enable PCIe controller support on Qualcomm SoCs. The
+	  PCIe controller use Designware core plus Qualcomm specific hardware
+	  wrappers.
 endmenu
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index f733b4e..a865294 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -15,3 +15,4 @@ obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
 obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o
 obj-$(CONFIG_PCIE_IPROC) += pcie-iproc.o
 obj-$(CONFIG_PCIE_IPROC_PLATFORM) += pcie-iproc-platform.o
+obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
diff --git a/drivers/pci/host/pcie-qcom.c b/drivers/pci/host/pcie-qcom.c
new file mode 100644
index 0000000..4f083c6
--- /dev/null
+++ b/drivers/pci/host/pcie-qcom.c
@@ -0,0 +1,677 @@
+/*
+ * Copyright (c) 2014, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_gpio.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/phy/phy.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#include "pcie-designware.h"
+
+#define PCIE20_PARF_PHY_CTRL			0x40
+#define PCIE20_PARF_PHY_REFCLK			0x4C
+#define PCIE20_PARF_DBI_BASE_ADDR		0x168
+#define PCIE20_PARF_SLV_ADDR_SPACE_SIZE		0x16c
+#define PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT	0x178
+
+#define PCIE20_ELBI_SYS_CTRL			0x04
+#define PCIE20_ELBI_SYS_STTS			0x08
+#define XMLH_LINK_UP				BIT(10)
+
+#define PCIE20_CAP				0x70
+#define PCIE20_CAP_LINKCTRLSTATUS		(PCIE20_CAP + 0x10)
+
+#define PERST_DELAY_MIN_US			1000
+#define PERST_DELAY_MAX_US			1005
+
+#define LINKUP_DELAY_MIN_US			5000
+#define LINKUP_DELAY_MAX_US			5100
+#define LINKUP_RETRIES_COUNT			20
+
+#define PCIE_V0					0	/* apq8064 */
+#define PCIE_V1					1	/* apq8084 */
+
+struct qcom_pcie_resources_v0 {
+	struct clk *iface_clk;
+	struct clk *core_clk;
+	struct clk *phy_clk;
+	struct reset_control *pci_reset;
+	struct reset_control *axi_reset;
+	struct reset_control *ahb_reset;
+	struct reset_control *por_reset;
+	struct reset_control *phy_reset;
+	struct regulator *vdda;
+	struct regulator *vdda_phy;
+	struct regulator *vdda_refclk;
+};
+
+struct qcom_pcie_resources_v1 {
+	struct clk *iface;
+	struct clk *aux;
+	struct clk *master_bus;
+	struct clk *slave_bus;
+	struct reset_control *core;
+	struct regulator *vdda;
+};
+
+union pcie_resources {
+	struct qcom_pcie_resources_v0 v0;
+	struct qcom_pcie_resources_v1 v1;
+};
+
+struct qcom_pcie {
+	struct pcie_port pp;
+	struct device *dev;
+	union pcie_resources res;
+	void __iomem *parf;
+	void __iomem *dbi;
+	void __iomem *elbi;
+	struct phy *phy;
+	struct gpio_desc *reset;
+	unsigned int version;
+};
+
+#define to_qcom_pcie(x)		container_of(x, struct qcom_pcie, pp)
+
+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_deassert(struct qcom_pcie *pcie, int assert)
+{
+	int val, active_low;
+
+	if (IS_ERR_OR_NULL(pcie->reset))
+		return;
+
+	active_low = gpiod_is_active_low(pcie->reset);
+
+	if (assert)
+		val = !!active_low;
+	else
+		val = !active_low;
+
+	gpiod_set_value(pcie->reset, val);
+
+	usleep_range(PERST_DELAY_MIN_US, PERST_DELAY_MAX_US);
+}
+
+static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
+{
+	qcom_ep_reset_assert_deassert(pcie, 1);
+}
+
+static void qcom_ep_reset_deassert(struct qcom_pcie *pcie)
+{
+	qcom_ep_reset_assert_deassert(pcie, 0);
+}
+
+static irqreturn_t qcom_pcie_msi_irq_handler(int irq, void *arg)
+{
+	struct pcie_port *pp = arg;
+
+	return dw_handle_msi_irq(pp);
+}
+
+static int qcom_pcie_link_up(struct pcie_port *pp)
+{
+	struct qcom_pcie *pcie = to_qcom_pcie(pp);
+	u32 val = readl(pcie->dbi + PCIE20_CAP_LINKCTRLSTATUS);
+
+	return val & BIT(29) ? 1 : 0;
+}
+
+static void qcom_pcie_disable_resources_v0(struct qcom_pcie *pcie)
+{
+	struct qcom_pcie_resources_v0 *res = &pcie->res.v0;
+
+	reset_control_assert(res->pci_reset);
+	reset_control_assert(res->axi_reset);
+	reset_control_assert(res->ahb_reset);
+	reset_control_assert(res->por_reset);
+	reset_control_assert(res->pci_reset);
+	clk_disable_unprepare(res->iface_clk);
+	clk_disable_unprepare(res->core_clk);
+	clk_disable_unprepare(res->phy_clk);
+	regulator_disable(res->vdda);
+	regulator_disable(res->vdda_phy);
+	regulator_disable(res->vdda_refclk);
+}
+
+static void qcom_pcie_disable_resources_v1(struct qcom_pcie *pcie)
+{
+	struct qcom_pcie_resources_v1 *res = &pcie->res.v1;
+
+	reset_control_assert(res->core);
+	clk_disable_unprepare(res->slave_bus);
+	clk_disable_unprepare(res->master_bus);
+	clk_disable_unprepare(res->iface);
+	clk_disable_unprepare(res->aux);
+	regulator_disable(res->vdda);
+}
+
+static int qcom_pcie_enable_resources_v0(struct qcom_pcie *pcie)
+{
+	struct qcom_pcie_resources_v0 *res = &pcie->res.v0;
+	struct device *dev = pcie->dev;
+	int ret;
+
+	ret = regulator_enable(res->vdda);
+	if (ret) {
+		dev_err(dev, "cannot enable vdda regulator\n");
+		return ret;
+	}
+
+	ret = regulator_enable(res->vdda_refclk);
+	if (ret) {
+		dev_err(dev, "cannot enable vdda_refclk regulator\n");
+		goto err_refclk;
+	}
+
+	ret = regulator_enable(res->vdda_phy);
+	if (ret) {
+		dev_err(dev, "cannot enable vdda_phy regulator\n");
+		goto err_vdda_phy;
+	}
+
+	ret = clk_prepare_enable(res->iface_clk);
+	if (ret) {
+		dev_err(dev, "cannot prepare/enable iface clock\n");
+		goto err_iface;
+	}
+
+	ret = clk_prepare_enable(res->core_clk);
+	if (ret) {
+		dev_err(dev, "cannot prepare/enable core clock\n");
+		goto err_clk_core;
+	}
+
+	ret = clk_prepare_enable(res->phy_clk);
+	if (ret) {
+		dev_err(dev, "cannot prepare/enable phy clock\n");
+		goto err_clk_phy;
+	}
+
+	ret = reset_control_deassert(res->ahb_reset);
+	if (ret) {
+		dev_err(dev, "cannot deassert ahb reset\n");
+		goto err_reset_ahb;
+	}
+
+	return 0;
+
+err_reset_ahb:
+	clk_disable_unprepare(res->phy_clk);
+err_clk_phy:
+	clk_disable_unprepare(res->core_clk);
+err_clk_core:
+	clk_disable_unprepare(res->iface_clk);
+err_iface:
+	regulator_disable(res->vdda_phy);
+err_vdda_phy:
+	regulator_disable(res->vdda_refclk);
+err_refclk:
+	regulator_disable(res->vdda);
+	return ret;
+}
+
+static int qcom_pcie_enable_resources_v1(struct qcom_pcie *pcie)
+{
+	struct qcom_pcie_resources_v1 *res = &pcie->res.v1;
+	struct device *dev = pcie->dev;
+	int ret;
+
+	ret = reset_control_deassert(res->core);
+	if (ret) {
+		dev_err(dev, "cannot deassert core reset\n");
+		return ret;
+	}
+
+	ret = clk_prepare_enable(res->aux);
+	if (ret) {
+		dev_err(dev, "cannot prepare/enable aux clock\n");
+		goto err_res;
+	}
+
+	ret = clk_prepare_enable(res->iface);
+	if (ret) {
+		dev_err(dev, "cannot prepare/enable iface clock\n");
+		goto err_aux;
+	}
+
+	ret = clk_prepare_enable(res->master_bus);
+	if (ret) {
+		dev_err(dev, "cannot prepare/enable master_bus clock\n");
+		goto err_iface;
+	}
+
+	ret = clk_prepare_enable(res->slave_bus);
+	if (ret) {
+		dev_err(dev, "cannot prepare/enable slave_bus clock\n");
+		goto err_master;
+	}
+
+	ret = regulator_enable(res->vdda);
+	if (ret) {
+		dev_err(dev, "cannot enable vdda regulator\n");
+		goto err_slave;
+	}
+
+	return 0;
+
+err_slave:
+	clk_disable_unprepare(res->slave_bus);
+err_master:
+	clk_disable_unprepare(res->master_bus);
+err_iface:
+	clk_disable_unprepare(res->iface);
+err_aux:
+	clk_disable_unprepare(res->aux);
+err_res:
+	reset_control_assert(res->core);
+
+	return ret;
+}
+
+static int qcom_pcie_get_resources_v0(struct qcom_pcie *pcie)
+{
+	struct qcom_pcie_resources_v0 *res = &pcie->res.v0;
+	struct device *dev = pcie->dev;
+
+	res->vdda = devm_regulator_get(dev, "vdda");
+	if (IS_ERR(res->vdda))
+		return PTR_ERR(res->vdda);
+
+	res->vdda_phy = devm_regulator_get(dev, "vdda_phy");
+	if (IS_ERR(res->vdda_phy))
+		return PTR_ERR(res->vdda_phy);
+
+	res->vdda_refclk = devm_regulator_get(dev, "vdda_refclk");
+	if (IS_ERR(res->vdda_refclk))
+		return PTR_ERR(res->vdda_refclk);
+
+	res->iface_clk = devm_clk_get(dev, "iface");
+	if (IS_ERR(res->iface_clk))
+		return PTR_ERR(res->iface_clk);
+
+	res->core_clk = devm_clk_get(dev, "core");
+	if (IS_ERR(res->core_clk))
+		return PTR_ERR(res->core_clk);
+
+	res->phy_clk = devm_clk_get(dev, "phy");
+	if (IS_ERR(res->phy_clk))
+		return PTR_ERR(res->phy_clk);
+
+	res->pci_reset = devm_reset_control_get(dev, "pci");
+	if (IS_ERR(res->pci_reset))
+		return PTR_ERR(res->pci_reset);
+
+	res->axi_reset = devm_reset_control_get(dev, "axi");
+	if (IS_ERR(res->axi_reset))
+		return PTR_ERR(res->axi_reset);
+
+	res->ahb_reset = devm_reset_control_get(dev, "ahb");
+	if (IS_ERR(res->ahb_reset))
+		return PTR_ERR(res->ahb_reset);
+
+	res->por_reset = devm_reset_control_get(dev, "por");
+	if (IS_ERR(res->por_reset))
+		return PTR_ERR(res->por_reset);
+
+	res->phy_reset = devm_reset_control_get(dev, "phy");
+	if (IS_ERR(res->phy_reset))
+		return PTR_ERR(res->phy_reset);
+
+	return 0;
+}
+
+static int qcom_pcie_get_resources_v1(struct qcom_pcie *pcie)
+{
+	struct qcom_pcie_resources_v1 *res = &pcie->res.v1;
+	struct device *dev = pcie->dev;
+
+	res->vdda = devm_regulator_get(dev, "vdda");
+	if (IS_ERR(res->vdda))
+		return PTR_ERR(res->vdda);
+
+	res->iface = devm_clk_get(dev, "iface");
+	if (IS_ERR(res->iface))
+		return PTR_ERR(res->iface);
+
+	res->aux = devm_clk_get(dev, "aux");
+	if (IS_ERR(res->aux) && PTR_ERR(res->aux) == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
+	else if (IS_ERR(res->aux))
+		res->aux = NULL;
+
+	res->master_bus = devm_clk_get(dev, "master_bus");
+	if (IS_ERR(res->master_bus))
+		return PTR_ERR(res->master_bus);
+
+	res->slave_bus = devm_clk_get(dev, "slave_bus");
+	if (IS_ERR(res->slave_bus))
+		return PTR_ERR(res->slave_bus);
+
+	res->core = devm_reset_control_get(dev, "core");
+	if (IS_ERR(res->core))
+		return PTR_ERR(res->core);
+
+	return 0;
+}
+
+static int qcom_pcie_enable_link_training(struct pcie_port *pp)
+{
+	struct qcom_pcie *pcie = to_qcom_pcie(pp);
+	struct device *dev = pp->dev;
+	int retries;
+	u32 val;
+
+	/* enable link training */
+	writel_masked(pcie->elbi + PCIE20_ELBI_SYS_CTRL, 0, BIT(0));
+
+	/* wait for up to 100ms for the link to come up */
+	retries = LINKUP_RETRIES_COUNT;
+	do {
+		val = readl(pcie->elbi + PCIE20_ELBI_SYS_STTS);
+		if (val & XMLH_LINK_UP)
+			break;
+		usleep_range(LINKUP_DELAY_MIN_US, LINKUP_DELAY_MAX_US);
+	} while (retries--);
+
+	if (retries < 0 || !dw_pcie_link_up(pp)) {
+		dev_err(dev, "link initialization failed\n");
+		return -ENXIO;
+	}
+
+	return 0;
+}
+
+static void qcom_pcie_host_init_v1(struct pcie_port *pp)
+{
+	struct qcom_pcie *pcie = to_qcom_pcie(pp);
+	int ret;
+
+	qcom_ep_reset_assert(pcie);
+
+	ret = qcom_pcie_enable_resources_v1(pcie);
+	if (ret)
+		return;
+
+	/* change DBI base address */
+	writel(0, pcie->parf + PCIE20_PARF_DBI_BASE_ADDR);
+
+	if (IS_ENABLED(CONFIG_PCI_MSI))
+		writel_masked(pcie->parf + PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT,
+			      0, BIT(31));
+
+	ret = phy_init(pcie->phy);
+	if (ret)
+		goto err_res;
+
+	ret = phy_power_on(pcie->phy);
+	if (ret)
+		goto err_phy;
+
+	dw_pcie_setup_rc(pp);
+
+	if (IS_ENABLED(CONFIG_PCI_MSI))
+		dw_pcie_msi_init(pp);
+
+	qcom_ep_reset_deassert(pcie);
+
+	ret = qcom_pcie_enable_link_training(pp);
+	if (ret)
+		goto err;
+
+	return;
+
+err:
+	qcom_ep_reset_assert(pcie);
+	phy_power_off(pcie->phy);
+err_phy:
+	phy_exit(pcie->phy);
+err_res:
+	qcom_pcie_disable_resources_v1(pcie);
+}
+
+static void qcom_pcie_host_init_v0(struct pcie_port *pp)
+{
+	struct qcom_pcie *pcie = to_qcom_pcie(pp);
+	struct qcom_pcie_resources_v0 *res = &pcie->res.v0;
+	struct device *dev = pcie->dev;
+	int ret;
+
+	qcom_ep_reset_assert(pcie);
+
+	ret = qcom_pcie_enable_resources_v0(pcie);
+	if (ret)
+		return;
+
+	writel_masked(pcie->parf + PCIE20_PARF_PHY_CTRL, BIT(0), 0);
+
+	/* enable external reference clock */
+	writel_masked(pcie->parf + PCIE20_PARF_PHY_REFCLK, 0, BIT(16));
+
+	ret = reset_control_deassert(res->phy_reset);
+	if (ret) {
+		dev_err(dev, "cannot deassert phy reset\n");
+		return;
+	}
+
+	ret = reset_control_deassert(res->pci_reset);
+	if (ret) {
+		dev_err(dev, "cannot deassert pci reset\n");
+		return;
+	}
+
+	ret = reset_control_deassert(res->por_reset);
+	if (ret) {
+		dev_err(dev, "cannot deassert por reset\n");
+		return;
+	}
+
+	ret = reset_control_deassert(res->axi_reset);
+	if (ret) {
+		dev_err(dev, "cannot deassert axi reset\n");
+		return;
+	}
+
+	/* wait 150ms for clock acquisition */
+	usleep_range(10000, 15000);
+
+	dw_pcie_setup_rc(pp);
+
+	if (IS_ENABLED(CONFIG_PCI_MSI))
+		dw_pcie_msi_init(pp);
+
+	qcom_ep_reset_deassert(pcie);
+
+	ret = qcom_pcie_enable_link_training(pp);
+	if (ret)
+		goto err;
+
+	return;
+err:
+	qcom_ep_reset_assert(pcie);
+	qcom_pcie_disable_resources_v0(pcie);
+}
+
+static void qcom_pcie_host_init(struct pcie_port *pp)
+{
+	struct qcom_pcie *pcie = to_qcom_pcie(pp);
+
+	if (pcie->version == PCIE_V0)
+		return qcom_pcie_host_init_v0(pp);
+	else
+		return qcom_pcie_host_init_v1(pp);
+}
+
+static int
+qcom_pcie_rd_own_conf(struct pcie_port *pp, int where, int size, u32 *val)
+{
+	/* the device class is not reported correctly from the register */
+	if (where == PCI_CLASS_REVISION && size == 4) {
+		*val = readl(pp->dbi_base + PCI_CLASS_REVISION);
+		*val &= ~(0xffff << 16);
+		*val |= PCI_CLASS_BRIDGE_PCI << 16;
+		return PCIBIOS_SUCCESSFUL;
+	}
+
+	return dw_pcie_cfg_read(pp->dbi_base + (where & ~0x3), where,
+				size, val);
+}
+
+static struct pcie_host_ops qcom_pcie_ops = {
+	.link_up = qcom_pcie_link_up,
+	.host_init = qcom_pcie_host_init,
+	.rd_own_conf = qcom_pcie_rd_own_conf,
+};
+
+static const struct of_device_id qcom_pcie_match[] = {
+	{ .compatible = "qcom,pcie-v0", .data = (void *)PCIE_V0 },
+	{ .compatible = "qcom,pcie-v1", .data = (void *)PCIE_V1 },
+	{ }
+};
+
+static int qcom_pcie_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct of_device_id *match;
+	struct resource *res;
+	struct qcom_pcie *pcie;
+	struct pcie_port *pp;
+	int ret;
+
+	match = of_match_node(qcom_pcie_match, dev->of_node);
+	if (!match)
+		return -ENXIO;
+
+	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
+	if (!pcie)
+		return -ENOMEM;
+
+	pcie->version = (unsigned int)match->data;
+
+	pcie->reset = devm_gpiod_get_optional(dev, "perst");
+	if (IS_ERR(pcie->reset) && PTR_ERR(pcie->reset) == -EPROBE_DEFER)
+		return PTR_ERR(pcie->reset);
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "parf");
+	pcie->parf = devm_ioremap_resource(dev, res);
+	if (IS_ERR(pcie->parf))
+		return PTR_ERR(pcie->parf);
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
+	pcie->dbi = devm_ioremap_resource(dev, res);
+	if (IS_ERR(pcie->dbi))
+		return PTR_ERR(pcie->dbi);
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "elbi");
+	pcie->elbi = devm_ioremap_resource(dev, res);
+	if (IS_ERR(pcie->elbi))
+		return PTR_ERR(pcie->elbi);
+
+	pcie->phy = devm_phy_optional_get(dev, "pciephy");
+	if (IS_ERR(pcie->phy))
+		return PTR_ERR(pcie->phy);
+
+	pcie->dev = dev;
+
+	if (pcie->version == PCIE_V0)
+		ret = qcom_pcie_get_resources_v0(pcie);
+	else
+		ret = qcom_pcie_get_resources_v1(pcie);
+
+	if (ret)
+		return ret;
+
+	pp = &pcie->pp;
+	pp->dev = dev;
+	pp->dbi_base = pcie->dbi;
+	pp->root_bus_nr = -1;
+	pp->ops = &qcom_pcie_ops;
+
+	if (IS_ENABLED(CONFIG_PCI_MSI)) {
+		pp->msi_irq = platform_get_irq_byname(pdev, "msi");
+		if (pp->msi_irq < 0) {
+			dev_err(dev, "cannot get msi irq\n");
+			return pp->msi_irq;
+		}
+
+		ret = devm_request_irq(dev, pp->msi_irq,
+				       qcom_pcie_msi_irq_handler,
+				       IRQF_SHARED, "qcom-pcie-msi", pp);
+		if (ret) {
+			dev_err(dev, "cannot request msi irq\n");
+			return ret;
+		}
+	}
+
+	ret = dw_pcie_host_init(pp);
+	if (ret) {
+		dev_err(dev, "cannot initialize host\n");
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, pcie);
+
+	return 0;
+}
+
+static int qcom_pcie_remove(struct platform_device *pdev)
+{
+	struct qcom_pcie *pcie = platform_get_drvdata(pdev);
+
+	qcom_ep_reset_assert(pcie);
+	phy_power_off(pcie->phy);
+	phy_exit(pcie->phy);
+	if (pcie->version == PCIE_V0)
+		qcom_pcie_disable_resources_v0(pcie);
+	else
+		qcom_pcie_disable_resources_v1(pcie);
+
+	return 0;
+}
+
+static struct platform_driver qcom_pcie_driver = {
+	.probe = qcom_pcie_probe,
+	.remove = qcom_pcie_remove,
+	.driver = {
+		.name = "qcom-pcie",
+		.of_match_table = qcom_pcie_match,
+	},
+};
+
+module_platform_driver(qcom_pcie_driver);
+
+MODULE_AUTHOR("Stanimir Varbanov <svarbanov@mm-sol.com>");
+MODULE_DESCRIPTION("Qualcomm PCIe root complex driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:qcom-pcie");
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 5/5] ARM: qcom: Add Qualcomm APQ8084 SoC
  2015-05-04 12:42 [PATCH v2 0/5] Qualcomm PCIe and PCIe/PHY drivers Stanimir Varbanov
                   ` (3 preceding siblings ...)
  2015-05-04 12:42 ` [PATCH v2 4/5] PCI: qcom: Add Qualcomm PCIe controller driver Stanimir Varbanov
@ 2015-05-04 12:42 ` Stanimir Varbanov
  2015-05-22 18:22   ` Stephen Boyd
  4 siblings, 1 reply; 17+ messages in thread
From: Stanimir Varbanov @ 2015-05-04 12:42 UTC (permalink / raw)
  To: Rob Herring, Kumar Gala, Mark Rutland, Grant Likely,
	Bjorn Helgaas, Kishon Vijay Abraham I, Russell King,
	Arnd Bergmann
  Cc: linux-arm-msm, linux-kernel, linux-arm-kernel, devicetree,
	linux-pci, Mathieu Olivari, Srinivas Kandagatla,
	Stanimir Varbanov

This adds Snapdragon 805 ARM-based SoC as multiplatform
compatible platform.

Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
---
 arch/arm/mach-qcom/Kconfig |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-qcom/Kconfig b/arch/arm/mach-qcom/Kconfig
index 2256cd1..9cfebee 100644
--- a/arch/arm/mach-qcom/Kconfig
+++ b/arch/arm/mach-qcom/Kconfig
@@ -22,4 +22,11 @@ config ARCH_MSM8974
 	bool "Enable support for MSM8974"
 	select HAVE_ARM_ARCH_TIMER
 
+config ARCH_APQ8084
+	bool "Enable support for APQ8084"
+	select HAVE_ARM_ARCH_TIMER
+	select MIGHT_HAVE_PCI
+	select PCI_DOMAINS if PCI
+	select PCI_MSI if PCI
+
 endif
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/5] phy: qcom: Add Qualcomm PCIe PHY
  2015-05-04 12:42 ` [PATCH v2 2/5] phy: qcom: Add Qualcomm PCIe PHY Stanimir Varbanov
@ 2015-05-04 14:35   ` Kishon Vijay Abraham I
  2015-05-04 15:24     ` Stanimir Varbanov
  0 siblings, 1 reply; 17+ messages in thread
From: Kishon Vijay Abraham I @ 2015-05-04 14:35 UTC (permalink / raw)
  To: Stanimir Varbanov, Rob Herring, Kumar Gala, Mark Rutland,
	Grant Likely, Bjorn Helgaas, Russell King, Arnd Bergmann
  Cc: linux-arm-msm, linux-kernel, linux-arm-kernel, devicetree,
	linux-pci, Mathieu Olivari, Srinivas Kandagatla

Hi,

On Monday 04 May 2015 06:12 PM, Stanimir Varbanov wrote:
> Add a PCIe PHY driver used by PCIe host controller driver
> on Qualcomm SoCs like Snapdragon 805.
>
> Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
> ---
>   drivers/phy/Kconfig         |    9 ++
>   drivers/phy/Makefile        |    1 +
>   drivers/phy/phy-qcom-pcie.c |  291 +++++++++++++++++++++++++++++++++++++++++++

Why do you need a new PHY driver for this? Why not use the existing QCOM PHY 
driver. I can see the registers used here in phy-qcom-ufs-qmp-14nm.h?

Thanks
Kishon


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/5] phy: qcom: Add Qualcomm PCIe PHY
  2015-05-04 14:35   ` Kishon Vijay Abraham I
@ 2015-05-04 15:24     ` Stanimir Varbanov
  2015-05-19 22:41       ` Bjorn Helgaas
  0 siblings, 1 reply; 17+ messages in thread
From: Stanimir Varbanov @ 2015-05-04 15:24 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Rob Herring, Kumar Gala, Mark Rutland, Grant Likely,
	Bjorn Helgaas, Russell King, Arnd Bergmann, linux-arm-msm,
	linux-kernel, linux-arm-kernel, devicetree, linux-pci,
	Mathieu Olivari, Srinivas Kandagatla

On 05/04/2015 05:35 PM, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Monday 04 May 2015 06:12 PM, Stanimir Varbanov wrote:
>> Add a PCIe PHY driver used by PCIe host controller driver
>> on Qualcomm SoCs like Snapdragon 805.
>>
>> Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
>> ---
>>   drivers/phy/Kconfig         |    9 ++
>>   drivers/phy/Makefile        |    1 +
>>   drivers/phy/phy-qcom-pcie.c |  291
>> +++++++++++++++++++++++++++++++++++++++++++
> 
> Why do you need a new PHY driver for this? Why not use the existing QCOM
> PHY driver. I can see the registers used here in phy-qcom-ufs-qmp-14nm.h?

I agree that on first glance there are similarities, but I'm not sure
does the PHYs are the same IP blocks. Or at least they are different
revisions which have too many differences. So trying to combine them
will lead to more code than now.

Either way I will try to understand how many the differences are.

-- 
regards,
Stan

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/5] phy: qcom: Add Qualcomm PCIe PHY
  2015-05-04 15:24     ` Stanimir Varbanov
@ 2015-05-19 22:41       ` Bjorn Helgaas
  2015-05-20 13:08         ` Kishon Vijay Abraham I
  0 siblings, 1 reply; 17+ messages in thread
From: Bjorn Helgaas @ 2015-05-19 22:41 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: Kishon Vijay Abraham I, Rob Herring, Kumar Gala, Mark Rutland,
	Grant Likely, Russell King, Arnd Bergmann, linux-arm-msm,
	linux-kernel, linux-arm-kernel, devicetree, linux-pci,
	Mathieu Olivari, Srinivas Kandagatla

On Mon, May 04, 2015 at 06:24:10PM +0300, Stanimir Varbanov wrote:
> On 05/04/2015 05:35 PM, Kishon Vijay Abraham I wrote:
> > Hi,
> > 
> > On Monday 04 May 2015 06:12 PM, Stanimir Varbanov wrote:
> >> Add a PCIe PHY driver used by PCIe host controller driver
> >> on Qualcomm SoCs like Snapdragon 805.
> >>
> >> Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
> >> ---
> >>   drivers/phy/Kconfig         |    9 ++
> >>   drivers/phy/Makefile        |    1 +
> >>   drivers/phy/phy-qcom-pcie.c |  291
> >> +++++++++++++++++++++++++++++++++++++++++++
> > 
> > Why do you need a new PHY driver for this? Why not use the existing QCOM
> > PHY driver. I can see the registers used here in phy-qcom-ufs-qmp-14nm.h?
> 
> I agree that on first glance there are similarities, but I'm not sure
> does the PHYs are the same IP blocks. Or at least they are different
> revisions which have too many differences. So trying to combine them
> will lead to more code than now.
> 
> Either way I will try to understand how many the differences are.

Ping, where are we with this?  Should I wait for something else, or are you
convinced there's enough difference to warrant a new PHY driver, Kishon?

Bjorn

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/5] phy: qcom: Add Qualcomm PCIe PHY
  2015-05-19 22:41       ` Bjorn Helgaas
@ 2015-05-20 13:08         ` Kishon Vijay Abraham I
  2015-05-20 13:23           ` Bjorn Helgaas
  0 siblings, 1 reply; 17+ messages in thread
From: Kishon Vijay Abraham I @ 2015-05-20 13:08 UTC (permalink / raw)
  To: Bjorn Helgaas, Stanimir Varbanov
  Cc: Rob Herring, Kumar Gala, Mark Rutland, Grant Likely,
	Russell King, Arnd Bergmann, linux-arm-msm, linux-kernel,
	linux-arm-kernel, devicetree, linux-pci, Mathieu Olivari,
	Srinivas Kandagatla

Hi Bjorn,

On Wednesday 20 May 2015 04:11 AM, Bjorn Helgaas wrote:
> On Mon, May 04, 2015 at 06:24:10PM +0300, Stanimir Varbanov wrote:
>> On 05/04/2015 05:35 PM, Kishon Vijay Abraham I wrote:
>>> Hi,
>>>
>>> On Monday 04 May 2015 06:12 PM, Stanimir Varbanov wrote:
>>>> Add a PCIe PHY driver used by PCIe host controller driver
>>>> on Qualcomm SoCs like Snapdragon 805.
>>>>
>>>> Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
>>>> ---
>>>>    drivers/phy/Kconfig         |    9 ++
>>>>    drivers/phy/Makefile        |    1 +
>>>>    drivers/phy/phy-qcom-pcie.c |  291
>>>> +++++++++++++++++++++++++++++++++++++++++++
>>>
>>> Why do you need a new PHY driver for this? Why not use the existing QCOM
>>> PHY driver. I can see the registers used here in phy-qcom-ufs-qmp-14nm.h?
>>
>> I agree that on first glance there are similarities, but I'm not sure
>> does the PHYs are the same IP blocks. Or at least they are different
>> revisions which have too many differences. So trying to combine them
>> will lead to more code than now.
>>
>> Either way I will try to understand how many the differences are.
>
> Ping, where are we with this?  Should I wait for something else, or are you
> convinced there's enough difference to warrant a new PHY driver, Kishon?

I'd like to wait to see if Stanimir can use existing driver instead of creating 
a new driver.

Cheers
Kishon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/5] phy: qcom: Add Qualcomm PCIe PHY
  2015-05-20 13:08         ` Kishon Vijay Abraham I
@ 2015-05-20 13:23           ` Bjorn Helgaas
  2015-05-22 16:25             ` Stanimir Varbanov
  0 siblings, 1 reply; 17+ messages in thread
From: Bjorn Helgaas @ 2015-05-20 13:23 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Stanimir Varbanov, Rob Herring, Kumar Gala, Mark Rutland,
	Grant Likely, Russell King, Arnd Bergmann, linux-arm-msm,
	linux-kernel, linux-arm, devicetree, linux-pci, Mathieu Olivari,
	Srinivas Kandagatla

On Wed, May 20, 2015 at 8:08 AM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
> Hi Bjorn,
>
>
> On Wednesday 20 May 2015 04:11 AM, Bjorn Helgaas wrote:
>>
>> On Mon, May 04, 2015 at 06:24:10PM +0300, Stanimir Varbanov wrote:
>>>
>>> On 05/04/2015 05:35 PM, Kishon Vijay Abraham I wrote:
>>>>
>>>> Hi,
>>>>
>>>> On Monday 04 May 2015 06:12 PM, Stanimir Varbanov wrote:
>>>>>
>>>>> Add a PCIe PHY driver used by PCIe host controller driver
>>>>> on Qualcomm SoCs like Snapdragon 805.
>>>>>
>>>>> Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
>>>>> ---
>>>>>    drivers/phy/Kconfig         |    9 ++
>>>>>    drivers/phy/Makefile        |    1 +
>>>>>    drivers/phy/phy-qcom-pcie.c |  291
>>>>> +++++++++++++++++++++++++++++++++++++++++++
>>>>
>>>>
>>>> Why do you need a new PHY driver for this? Why not use the existing QCOM
>>>> PHY driver. I can see the registers used here in
>>>> phy-qcom-ufs-qmp-14nm.h?
>>>
>>>
>>> I agree that on first glance there are similarities, but I'm not sure
>>> does the PHYs are the same IP blocks. Or at least they are different
>>> revisions which have too many differences. So trying to combine them
>>> will lead to more code than now.
>>>
>>> Either way I will try to understand how many the differences are.
>>
>>
>> Ping, where are we with this?  Should I wait for something else, or are
>> you
>> convinced there's enough difference to warrant a new PHY driver, Kishon?
>
>
> I'd like to wait to see if Stanimir can use existing driver instead of
> creating a new driver.

OK, I'm waiting for a v3 with either tweaks to the QCOM PHY driver, or
stronger justification as to why that's impossible.  It's hard to
quantify statements like "there are similarities, but there are too
many differences," so you might have to actually attempt a patch for
the QCOM code and we can see how ugly that would turn out to be.

Thanks, Kishon.

Bjorn

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/5] phy: qcom: Add Qualcomm PCIe PHY
  2015-05-20 13:23           ` Bjorn Helgaas
@ 2015-05-22 16:25             ` Stanimir Varbanov
  2015-05-22 18:06               ` Bjorn Helgaas
  0 siblings, 1 reply; 17+ messages in thread
From: Stanimir Varbanov @ 2015-05-22 16:25 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Kishon Vijay Abraham I, Rob Herring, Kumar Gala, Mark Rutland,
	Grant Likely, Russell King, Arnd Bergmann, linux-arm-msm,
	linux-kernel, linux-arm, devicetree, linux-pci, Mathieu Olivari,
	Srinivas Kandagatla

On 05/20/2015 04:23 PM, Bjorn Helgaas wrote:
> On Wed, May 20, 2015 at 8:08 AM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>> Hi Bjorn,
>>
>>
>> On Wednesday 20 May 2015 04:11 AM, Bjorn Helgaas wrote:
>>>
>>> On Mon, May 04, 2015 at 06:24:10PM +0300, Stanimir Varbanov wrote:
>>>>
>>>> On 05/04/2015 05:35 PM, Kishon Vijay Abraham I wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> On Monday 04 May 2015 06:12 PM, Stanimir Varbanov wrote:
>>>>>>
>>>>>> Add a PCIe PHY driver used by PCIe host controller driver
>>>>>> on Qualcomm SoCs like Snapdragon 805.
>>>>>>
>>>>>> Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
>>>>>> ---
>>>>>>    drivers/phy/Kconfig         |    9 ++
>>>>>>    drivers/phy/Makefile        |    1 +
>>>>>>    drivers/phy/phy-qcom-pcie.c |  291
>>>>>> +++++++++++++++++++++++++++++++++++++++++++
>>>>>
>>>>>
>>>>> Why do you need a new PHY driver for this? Why not use the existing QCOM
>>>>> PHY driver. I can see the registers used here in
>>>>> phy-qcom-ufs-qmp-14nm.h?
>>>>
>>>>
>>>> I agree that on first glance there are similarities, but I'm not sure
>>>> does the PHYs are the same IP blocks. Or at least they are different
>>>> revisions which have too many differences. So trying to combine them
>>>> will lead to more code than now.
>>>>
>>>> Either way I will try to understand how many the differences are.
>>>
>>>
>>> Ping, where are we with this?  Should I wait for something else, or are
>>> you
>>> convinced there's enough difference to warrant a new PHY driver, Kishon?
>>
>>
>> I'd like to wait to see if Stanimir can use existing driver instead of
>> creating a new driver.
> 
> OK, I'm waiting for a v3 with either tweaks to the QCOM PHY driver, or
> stronger justification as to why that's impossible.  It's hard to
> quantify statements like "there are similarities, but there are too
> many differences," so you might have to actually attempt a patch for
> the QCOM code and we can see how ugly that would turn out to be.

Kishon,

I'm still checking the differences, sorry for the delay.

Bjorn, is the pcie driver looks good enough to be merged?

-- 
regards,
Stan

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 2/5] phy: qcom: Add Qualcomm PCIe PHY
  2015-05-22 16:25             ` Stanimir Varbanov
@ 2015-05-22 18:06               ` Bjorn Helgaas
  0 siblings, 0 replies; 17+ messages in thread
From: Bjorn Helgaas @ 2015-05-22 18:06 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: Kishon Vijay Abraham I, Rob Herring, Kumar Gala, Mark Rutland,
	Grant Likely, Russell King, Arnd Bergmann, linux-arm-msm,
	linux-kernel, linux-arm, devicetree, linux-pci, Mathieu Olivari,
	Srinivas Kandagatla, Jingoo Han, Pratyush Anand

[+cc Jingoo, Pratyush]

On Fri, May 22, 2015 at 11:25 AM, Stanimir Varbanov
<svarbanov@mm-sol.com> wrote:
> On 05/20/2015 04:23 PM, Bjorn Helgaas wrote:
>> On Wed, May 20, 2015 at 8:08 AM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>>> Hi Bjorn,
>>>
>>>
>>> On Wednesday 20 May 2015 04:11 AM, Bjorn Helgaas wrote:
>>>>
>>>> On Mon, May 04, 2015 at 06:24:10PM +0300, Stanimir Varbanov wrote:
>>>>>
>>>>> On 05/04/2015 05:35 PM, Kishon Vijay Abraham I wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On Monday 04 May 2015 06:12 PM, Stanimir Varbanov wrote:
>>>>>>>
>>>>>>> Add a PCIe PHY driver used by PCIe host controller driver
>>>>>>> on Qualcomm SoCs like Snapdragon 805.
>>>>>>>
>>>>>>> Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
>>>>>>> ---
>>>>>>>    drivers/phy/Kconfig         |    9 ++
>>>>>>>    drivers/phy/Makefile        |    1 +
>>>>>>>    drivers/phy/phy-qcom-pcie.c |  291
>>>>>>> +++++++++++++++++++++++++++++++++++++++++++
>>>>>>
>>>>>>
>>>>>> Why do you need a new PHY driver for this? Why not use the existing QCOM
>>>>>> PHY driver. I can see the registers used here in
>>>>>> phy-qcom-ufs-qmp-14nm.h?
>>>>>
>>>>>
>>>>> I agree that on first glance there are similarities, but I'm not sure
>>>>> does the PHYs are the same IP blocks. Or at least they are different
>>>>> revisions which have too many differences. So trying to combine them
>>>>> will lead to more code than now.
>>>>>
>>>>> Either way I will try to understand how many the differences are.
>>>>
>>>>
>>>> Ping, where are we with this?  Should I wait for something else, or are
>>>> you
>>>> convinced there's enough difference to warrant a new PHY driver, Kishon?
>>>
>>>
>>> I'd like to wait to see if Stanimir can use existing driver instead of
>>> creating a new driver.
>>
>> OK, I'm waiting for a v3 with either tweaks to the QCOM PHY driver, or
>> stronger justification as to why that's impossible.  It's hard to
>> quantify statements like "there are similarities, but there are too
>> many differences," so you might have to actually attempt a patch for
>> the QCOM code and we can see how ugly that would turn out to be.
>
> Kishon,
>
> I'm still checking the differences, sorry for the delay.
>
> Bjorn, is the pcie driver looks good enough to be merged?

I haven't looked at that (I was waiting until I could deal with the
whole series).  But it sounds like it would be useful to merge patches
3-5 by themselves?  I'll take a look at those.

It would be good to get Arnd and Rob to take a look, especially at the DT parts.

And Jingoo and Pratyush since you use the DesignWare core.

Bjorn

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 5/5] ARM: qcom: Add Qualcomm APQ8084 SoC
  2015-05-04 12:42 ` [PATCH v2 5/5] ARM: qcom: Add Qualcomm APQ8084 SoC Stanimir Varbanov
@ 2015-05-22 18:22   ` Stephen Boyd
  2015-05-22 19:26     ` Arnd Bergmann
  0 siblings, 1 reply; 17+ messages in thread
From: Stephen Boyd @ 2015-05-22 18:22 UTC (permalink / raw)
  To: Stanimir Varbanov, Rob Herring, Kumar Gala, Mark Rutland,
	Grant Likely, Bjorn Helgaas, Kishon Vijay Abraham I,
	Russell King, Arnd Bergmann
  Cc: linux-arm-msm, linux-kernel, linux-arm-kernel, devicetree,
	linux-pci, Mathieu Olivari, Srinivas Kandagatla

On 05/04/15 05:42, Stanimir Varbanov wrote:
> This adds Snapdragon 805 ARM-based SoC as multiplatform
> compatible platform.
>
> Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
> ---
>  arch/arm/mach-qcom/Kconfig |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-qcom/Kconfig b/arch/arm/mach-qcom/Kconfig
> index 2256cd1..9cfebee 100644
> --- a/arch/arm/mach-qcom/Kconfig
> +++ b/arch/arm/mach-qcom/Kconfig
> @@ -22,4 +22,11 @@ config ARCH_MSM8974
>  	bool "Enable support for MSM8974"
>  	select HAVE_ARM_ARCH_TIMER
>  
> +config ARCH_APQ8084
> +	bool "Enable support for APQ8084"
> +	select HAVE_ARM_ARCH_TIMER
> +	select MIGHT_HAVE_PCI
> +	select PCI_DOMAINS if PCI
> +	select PCI_MSI if PCI
> +
>

Can we move these PCI selects up to ARCH_QCOM directly? Let's try to
avoid adding more SoC specific options under ARCH_QCOM.

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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 5/5] ARM: qcom: Add Qualcomm APQ8084 SoC
  2015-05-22 18:22   ` Stephen Boyd
@ 2015-05-22 19:26     ` Arnd Bergmann
  0 siblings, 0 replies; 17+ messages in thread
From: Arnd Bergmann @ 2015-05-22 19:26 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Stephen Boyd, Stanimir Varbanov, Rob Herring, Kumar Gala,
	Mark Rutland, Grant Likely, Bjorn Helgaas,
	Kishon Vijay Abraham I, Russell King, devicetree, linux-arm-msm,
	linux-kernel, Srinivas Kandagatla, Mathieu Olivari, linux-pci

On Friday 22 May 2015 11:22:52 Stephen Boyd wrote:
> > diff --git a/arch/arm/mach-qcom/Kconfig b/arch/arm/mach-qcom/Kconfig
> > index 2256cd1..9cfebee 100644
> > --- a/arch/arm/mach-qcom/Kconfig
> > +++ b/arch/arm/mach-qcom/Kconfig
> > @@ -22,4 +22,11 @@ config ARCH_MSM8974
> >       bool "Enable support for MSM8974"
> >       select HAVE_ARM_ARCH_TIMER
> >  
> > +config ARCH_APQ8084
> > +     bool "Enable support for APQ8084"
> > +     select HAVE_ARM_ARCH_TIMER
> > +     select MIGHT_HAVE_PCI
> > +     select PCI_DOMAINS if PCI
> > +     select PCI_MSI if PCI
> > +
> >
> 
> Can we move these PCI selects up to ARCH_QCOM directly? Let's try to
> avoid adding more SoC specific options under ARCH_QCOM.

I thought we already select 'MIGHT_HAVE_PCI' and 'PCI_DOMAINS' for
all ARMv7 platforms. If not, we probably should.

For PCI_MSI, I would leave that as user-selectable, there is no
need to enforce that as far as I can tell. If the driver needs
PCI_MSI support at compile-time, a dependency for that driver
would make sense.

	Arnd

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 4/5] PCI: qcom: Add Qualcomm PCIe controller driver
  2015-05-04 12:42 ` [PATCH v2 4/5] PCI: qcom: Add Qualcomm PCIe controller driver Stanimir Varbanov
@ 2015-11-06 20:50   ` Bjorn Andersson
  2015-11-09 16:56     ` Stanimir Varbanov
  0 siblings, 1 reply; 17+ messages in thread
From: Bjorn Andersson @ 2015-11-06 20:50 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: Rob Herring, Kumar Gala, Mark Rutland, Grant Likely,
	Bjorn Helgaas, Kishon Vijay Abraham I, Russell King,
	Arnd Bergmann, linux-arm-msm, linux-kernel, linux-arm-kernel,
	devicetree, linux-pci, Mathieu Olivari, Srinivas Kandagatla

On Mon 04 May 05:42 PDT 2015, Stanimir Varbanov wrote:

> The PCIe driver reuse the Designware common code for host
> and MSI initialization, and also program the Qualcomm
> application specific registers.
> 

I want to get the ethernet on the ifc6410 running and this seems like
the last patchset for the PCIe.

> Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
[..]
> diff --git a/drivers/pci/host/pcie-qcom.c b/drivers/pci/host/pcie-qcom.c
> new file mode 100644
> index 0000000..4f083c6
> --- /dev/null
> +++ b/drivers/pci/host/pcie-qcom.c
> @@ -0,0 +1,677 @@
> +/*
> + * Copyright (c) 2014, The Linux Foundation. All rights reserved.

Bump the year, it's the future now :)

> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
[..]
> +
> +struct qcom_pcie {
> +	struct pcie_port pp;
> +	struct device *dev;

You already have this device pointer in pp->dev.

> +	union pcie_resources res;
> +	void __iomem *parf;
> +	void __iomem *dbi;
> +	void __iomem *elbi;
> +	struct phy *phy;
> +	struct gpio_desc *reset;
> +	unsigned int version;
> +};
> +
> +#define to_qcom_pcie(x)		container_of(x, struct qcom_pcie, pp)
> +
> +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);
> +}

There are no case where you do clear and set, so I would like that you
just inline this function where you need it. It adds 2 extra lines at a
few places, but a lot of clarity.

> +
> +static void qcom_ep_reset_assert_deassert(struct qcom_pcie *pcie, int assert)
> +{
> +	int val, active_low;
> +
> +	if (IS_ERR_OR_NULL(pcie->reset))
> +		return;

This will be NULL or valid here, never ERR. So it's fine to call
gpiod_set_value() with this pointer without the check.

> +
> +	active_low = gpiod_is_active_low(pcie->reset);
> +

gpiod_set_value() checks for FLAG_ACTIVE_LOW and will invert the value
first thing. So you do not need to do this manually.

> +	if (assert)
> +		val = !!active_low;
> +	else
> +		val = !active_low;
> +
> +	gpiod_set_value(pcie->reset, val);
> +
> +	usleep_range(PERST_DELAY_MIN_US, PERST_DELAY_MAX_US);

This doesn't seem to be called in a hot path, so you should be able to
simply msleep(1) here.

> +}
> +
> +static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
> +{
> +	qcom_ep_reset_assert_deassert(pcie, 1);

If we're down to this function just being a gpiod_set_value() and a
sleep we can inline it here instead of having this proxy function.

> +}
> +
> +static void qcom_ep_reset_deassert(struct qcom_pcie *pcie)
> +{
> +	qcom_ep_reset_assert_deassert(pcie, 0);

Same here.

> +}
> +
> +static irqreturn_t qcom_pcie_msi_irq_handler(int irq, void *arg)
> +{
> +	struct pcie_port *pp = arg;
> +
> +	return dw_handle_msi_irq(pp);
> +}
> +
> +static int qcom_pcie_link_up(struct pcie_port *pp)
> +{
> +	struct qcom_pcie *pcie = to_qcom_pcie(pp);
> +	u32 val = readl(pcie->dbi + PCIE20_CAP_LINKCTRLSTATUS);
> +
> +	return val & BIT(29) ? 1 : 0;

return !!(val & BIT(29));

Do we know what this bit 29 is?

> +}
> +
[..]
> +
> +static int qcom_pcie_get_resources_v1(struct qcom_pcie *pcie)
> +{
> +	struct qcom_pcie_resources_v1 *res = &pcie->res.v1;
> +	struct device *dev = pcie->dev;
> +
> +	res->vdda = devm_regulator_get(dev, "vdda");
> +	if (IS_ERR(res->vdda))
> +		return PTR_ERR(res->vdda);
> +
> +	res->iface = devm_clk_get(dev, "iface");
> +	if (IS_ERR(res->iface))
> +		return PTR_ERR(res->iface);
> +
> +	res->aux = devm_clk_get(dev, "aux");
> +	if (IS_ERR(res->aux) && PTR_ERR(res->aux) == -EPROBE_DEFER)

PTR_ERR() == x implies IS_ERR(), so you don't need both.

> +		return -EPROBE_DEFER;
> +	else if (IS_ERR(res->aux))
> +		res->aux = NULL;
> +
> +	res->master_bus = devm_clk_get(dev, "master_bus");
> +	if (IS_ERR(res->master_bus))
> +		return PTR_ERR(res->master_bus);
> +
> +	res->slave_bus = devm_clk_get(dev, "slave_bus");
> +	if (IS_ERR(res->slave_bus))
> +		return PTR_ERR(res->slave_bus);
> +
> +	res->core = devm_reset_control_get(dev, "core");
> +	if (IS_ERR(res->core))
> +		return PTR_ERR(res->core);
> +
> +	return 0;
> +}
> +
> +static int qcom_pcie_enable_link_training(struct pcie_port *pp)
> +{
> +	struct qcom_pcie *pcie = to_qcom_pcie(pp);
> +	struct device *dev = pp->dev;
> +	int retries;
> +	u32 val;
> +
> +	/* enable link training */
> +	writel_masked(pcie->elbi + PCIE20_ELBI_SYS_CTRL, 0, BIT(0));
> +
> +	/* wait for up to 100ms for the link to come up */
> +	retries = LINKUP_RETRIES_COUNT;
> +	do {
> +		val = readl(pcie->elbi + PCIE20_ELBI_SYS_STTS);
> +		if (val & XMLH_LINK_UP)
> +			break;
> +		usleep_range(LINKUP_DELAY_MIN_US, LINKUP_DELAY_MAX_US);
> +	} while (retries--);
> +
> +	if (retries < 0 || !dw_pcie_link_up(pp)) {
> +		dev_err(dev, "link initialization failed\n");
> +		return -ENXIO;
> +	}

Rather than implementing this based on a number of retries, I think it
would be cleaner to set a goal for jiffies + HZ / 10, then in the loop
you check your conditions, then check for time_after(jiffies, timeout)
and finally msleep() for a while.

> +
> +	return 0;
> +}
> +
[..]
> +static void qcom_pcie_host_init_v0(struct pcie_port *pp)
> +{
> +	struct qcom_pcie *pcie = to_qcom_pcie(pp);
> +	struct qcom_pcie_resources_v0 *res = &pcie->res.v0;
> +	struct device *dev = pcie->dev;
> +	int ret;
> +
> +	qcom_ep_reset_assert(pcie);
> +
> +	ret = qcom_pcie_enable_resources_v0(pcie);
> +	if (ret)
> +		return;
> +
> +	writel_masked(pcie->parf + PCIE20_PARF_PHY_CTRL, BIT(0), 0);

Do we have a name for this bit?

> +
> +	/* enable external reference clock */
> +	writel_masked(pcie->parf + PCIE20_PARF_PHY_REFCLK, 0, BIT(16));

Do we have a name for this bit?

> +
> +	ret = reset_control_deassert(res->phy_reset);
> +	if (ret) {
> +		dev_err(dev, "cannot deassert phy reset\n");
> +		return;
> +	}
> +
> +	ret = reset_control_deassert(res->pci_reset);
> +	if (ret) {
> +		dev_err(dev, "cannot deassert pci reset\n");
> +		return;
> +	}
> +
> +	ret = reset_control_deassert(res->por_reset);
> +	if (ret) {
> +		dev_err(dev, "cannot deassert por reset\n");
> +		return;
> +	}
> +
> +	ret = reset_control_deassert(res->axi_reset);
> +	if (ret) {
> +		dev_err(dev, "cannot deassert axi reset\n");
> +		return;
> +	}
> +
> +	/* wait 150ms for clock acquisition */
> +	usleep_range(10000, 15000);

Either the comment or the sleep is wrong. Also, I think you should just
use msleep() here, the precision doesn't look important.

> +
> +	dw_pcie_setup_rc(pp);
> +
> +	if (IS_ENABLED(CONFIG_PCI_MSI))
> +		dw_pcie_msi_init(pp);
> +
> +	qcom_ep_reset_deassert(pcie);
> +
> +	ret = qcom_pcie_enable_link_training(pp);
> +	if (ret)
> +		goto err;
> +
> +	return;
> +err:
> +	qcom_ep_reset_assert(pcie);
> +	qcom_pcie_disable_resources_v0(pcie);
> +}
> +
[..]
> +static int
> +qcom_pcie_rd_own_conf(struct pcie_port *pp, int where, int size, u32 *val)
> +{
> +	/* the device class is not reported correctly from the register */
> +	if (where == PCI_CLASS_REVISION && size == 4) {
> +		*val = readl(pp->dbi_base + PCI_CLASS_REVISION);
> +		*val &= ~(0xffff << 16);

Isn't this the same as *val &= 0xffff; ?

> +		*val |= PCI_CLASS_BRIDGE_PCI << 16;

Feels like it would be cleaner to just to:

u32 rev;

rev = readl(pp->dbi_base + PCI_CLASS_REVISION);
*val = PCI_CLASS_BRIDGE_PCI << 16 | rev & 0xffff;

> +		return PCIBIOS_SUCCESSFUL;
> +	}
> +
> +	return dw_pcie_cfg_read(pp->dbi_base + (where & ~0x3), where,
> +				size, val);

This api has been simplified, the new version should be:

  dw_pcie_cfg_read(pp->dbi_base + where, size, val);

> +}
> +
[..]
> +static const struct of_device_id qcom_pcie_match[] = {
> +	{ .compatible = "qcom,pcie-v0", .data = (void *)PCIE_V0 },
> +	{ .compatible = "qcom,pcie-v1", .data = (void *)PCIE_V1 },
> +	{ }
> +};

MODULE_DEVICE_TABLE() and move this below remove()

> +
> +static int qcom_pcie_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	const struct of_device_id *match;
> +	struct resource *res;
> +	struct qcom_pcie *pcie;
> +	struct pcie_port *pp;
> +	int ret;
> +
> +	match = of_match_node(qcom_pcie_match, dev->of_node);
> +	if (!match)
> +		return -ENXIO;
> +
> +	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
> +	if (!pcie)
> +		return -ENOMEM;
> +
> +	pcie->version = (unsigned int)match->data;

Use of_device_get_match_data() here instead.

> +
> +	pcie->reset = devm_gpiod_get_optional(dev, "perst");
> +	if (IS_ERR(pcie->reset) && PTR_ERR(pcie->reset) == -EPROBE_DEFER)
> +		return PTR_ERR(pcie->reset);

devm_gpiod_get_optional() will either return an IS_ERR() or NULL in the
case of it not being found. So I think you should always return
PTR_ERR() if IS_ERR() is set.


Also, the devm_gpiod_get_optional() api has gotten a third parameter,
set it to GPIOD_OUT_LOW.

> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "parf");
> +	pcie->parf = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(pcie->parf))
> +		return PTR_ERR(pcie->parf);
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
> +	pcie->dbi = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(pcie->dbi))
> +		return PTR_ERR(pcie->dbi);
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "elbi");
> +	pcie->elbi = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(pcie->elbi))
> +		return PTR_ERR(pcie->elbi);
> +
> +	pcie->phy = devm_phy_optional_get(dev, "pciephy");
> +	if (IS_ERR(pcie->phy))
> +		return PTR_ERR(pcie->phy);
> +
> +	pcie->dev = dev;

nit, move this lonely assignment up to the version assignment to make it
happier. Or you could just use pcie->pp->dev throughout the driver...

> +
> +	if (pcie->version == PCIE_V0)
> +		ret = qcom_pcie_get_resources_v0(pcie);
> +	else
> +		ret = qcom_pcie_get_resources_v1(pcie);
> +
> +	if (ret)
> +		return ret;
> +
> +	pp = &pcie->pp;
> +	pp->dev = dev;
> +	pp->dbi_base = pcie->dbi;
> +	pp->root_bus_nr = -1;
> +	pp->ops = &qcom_pcie_ops;
> +
> +	if (IS_ENABLED(CONFIG_PCI_MSI)) {
> +		pp->msi_irq = platform_get_irq_byname(pdev, "msi");
> +		if (pp->msi_irq < 0) {
> +			dev_err(dev, "cannot get msi irq\n");
> +			return pp->msi_irq;
> +		}
> +

Feeding an error value of msi_irq into request_irq will fails
gracefully, so you could just skip the error handling above.

> +		ret = devm_request_irq(dev, pp->msi_irq,
> +				       qcom_pcie_msi_irq_handler,
> +				       IRQF_SHARED, "qcom-pcie-msi", pp);
> +		if (ret) {
> +			dev_err(dev, "cannot request msi irq\n");
> +			return ret;
> +		}
> +	}
> +
> +	ret = dw_pcie_host_init(pp);
> +	if (ret) {
> +		dev_err(dev, "cannot initialize host\n");
> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, pcie);
> +
> +	return 0;
> +}
> +
[..]
> +
> +MODULE_AUTHOR("Stanimir Varbanov <svarbanov@mm-sol.com>");
> +MODULE_DESCRIPTION("Qualcomm PCIe root complex driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:qcom-pcie");

There's little point in the MODULE_ALIAS(), please drop it.

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 4/5] PCI: qcom: Add Qualcomm PCIe controller driver
  2015-11-06 20:50   ` Bjorn Andersson
@ 2015-11-09 16:56     ` Stanimir Varbanov
  0 siblings, 0 replies; 17+ messages in thread
From: Stanimir Varbanov @ 2015-11-09 16:56 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Rob Herring, Kumar Gala, Mark Rutland, Grant Likely,
	Bjorn Helgaas, Kishon Vijay Abraham I, Russell King,
	Arnd Bergmann, linux-arm-msm, linux-kernel, linux-arm-kernel,
	devicetree, linux-pci, Mathieu Olivari, Srinivas Kandagatla

Hi Bjorn,

On 11/06/2015 10:50 PM, Bjorn Andersson wrote:
> On Mon 04 May 05:42 PDT 2015, Stanimir Varbanov wrote:
> 
>> The PCIe driver reuse the Designware common code for host
>> and MSI initialization, and also program the Qualcomm
>> application specific registers.
>>
> 
> I want to get the ethernet on the ifc6410 running and this seems like
> the last patchset for the PCIe.
> 
>> Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
> [..]
>> diff --git a/drivers/pci/host/pcie-qcom.c b/drivers/pci/host/pcie-qcom.c
>> new file mode 100644
>> index 0000000..4f083c6
>> --- /dev/null
>> +++ b/drivers/pci/host/pcie-qcom.c
>> @@ -0,0 +1,677 @@
>> +/*
>> + * Copyright (c) 2014, The Linux Foundation. All rights reserved.
> 
> Bump the year, it's the future now :)

yep, thanks for the valuable comments I will address all of them on next
version.

I will probably restart the work on the driver this week.

-- 
regards,
Stan

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2015-11-09 16:57 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-04 12:42 [PATCH v2 0/5] Qualcomm PCIe and PCIe/PHY drivers Stanimir Varbanov
2015-05-04 12:42 ` [PATCH v2 1/5] DT: phy: qcom: Add PCIe PHY devicetree bindings Stanimir Varbanov
2015-05-04 12:42 ` [PATCH v2 2/5] phy: qcom: Add Qualcomm PCIe PHY Stanimir Varbanov
2015-05-04 14:35   ` Kishon Vijay Abraham I
2015-05-04 15:24     ` Stanimir Varbanov
2015-05-19 22:41       ` Bjorn Helgaas
2015-05-20 13:08         ` Kishon Vijay Abraham I
2015-05-20 13:23           ` Bjorn Helgaas
2015-05-22 16:25             ` Stanimir Varbanov
2015-05-22 18:06               ` Bjorn Helgaas
2015-05-04 12:42 ` [PATCH v2 3/5] DT: PCI: qcom: Document PCIe devicetree bindings Stanimir Varbanov
2015-05-04 12:42 ` [PATCH v2 4/5] PCI: qcom: Add Qualcomm PCIe controller driver Stanimir Varbanov
2015-11-06 20:50   ` Bjorn Andersson
2015-11-09 16:56     ` Stanimir Varbanov
2015-05-04 12:42 ` [PATCH v2 5/5] ARM: qcom: Add Qualcomm APQ8084 SoC Stanimir Varbanov
2015-05-22 18:22   ` Stephen Boyd
2015-05-22 19:26     ` Arnd Bergmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).