This patch set is continuation of previous v2 at [1]. This time I
decided to drop the PHY driver needed by pcie driver for apq8084
and send it separately soon (when sort out the need of separate phy
driver).
First two patches in the set are fixes in pcie designware driver
found during testing the qcom pcie driver. The second patch can
be treated as an RFC.
Next two patches adds DT binding document and the driver itself.
The last two patches just adds relevant dt node for apq8064 and
enable pcie usage on ifc6410 development board. The testing has been
made on ifc6410 development board.
Comments are welcome!
Here is edited description:
This patchset introduces a Qualcomm PCIe root complex driver for
Snapdragon 805 (APQ8084) and Snapdragon 600 (APQ8064). The PCIe
hardware use Designware IP core plus Qualcomm application
specific hw wrappers.
[1] https://lkml.org/lkml/2015/5/4/267
Stanimir Varbanov (6):
PCI: designware: remove wrong io_base assignment
PCI: designware: add memory barrier after enabling region
DT: PCI: qcom: Document PCIe devicetree bindings
PCI: qcom: Add Qualcomm PCIe controller driver
ARM: dts: apq8064: add pcie devicetree node
ARM: dts: ifc6410: enable pcie dt node for this board
.../devicetree/bindings/pci/qcom,pcie.txt | 231 ++++++++
MAINTAINERS | 7 +
arch/arm/boot/dts/qcom-apq8064-ifc6410.dts | 26 +
arch/arm/boot/dts/qcom-apq8064.dtsi | 36 ++
drivers/pci/host/Kconfig | 10 +
drivers/pci/host/Makefile | 1 +
drivers/pci/host/pcie-designware.c | 6 +-
drivers/pci/host/pcie-qcom.c | 623 ++++++++++++++++++++
8 files changed, 939 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/pci/qcom,pcie.txt
create mode 100644 drivers/pci/host/pcie-qcom.c
--
1.7.9.5
The io_base is used to keep the cpu physical address parsed
from ranges dt property. After issue pci_remap_iospace the
io_base has been assigned with io->start, which is not correct
cause io->start is a PCI bus address.
Signed-off-by: Stanimir Varbanov <[email protected]>
---
drivers/pci/host/pcie-designware.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 540f077c37ea..02a7452bdf23 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -440,7 +440,6 @@ int dw_pcie_host_init(struct pcie_port *pp)
ret, pp->io);
continue;
}
- pp->io_base = pp->io->start;
break;
case IORESOURCE_MEM:
pp->mem = win->res;
--
1.7.9.5
Add 'write memory' barrier after enable region in PCIE_ATU_CR2
register. The barrier is needed to ensure that the region enable
request has been reached it's destination at time when we
read/write to PCI configuration space.
Without this barrier PCI device enumeration during kernel boot
is not reliable, and reading configuration space for particular
PCI device on the bus returns zero aka no device.
Signed-off-by: Stanimir Varbanov <[email protected]>
---
drivers/pci/host/pcie-designware.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 02a7452bdf23..e15a2ae1583f 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -164,6 +164,11 @@ static void dw_pcie_prog_outbound_atu(struct pcie_port *pp, int index,
dw_pcie_writel_rc(pp, upper_32_bits(pci_addr), PCIE_ATU_UPPER_TARGET);
dw_pcie_writel_rc(pp, type, PCIE_ATU_CR1);
dw_pcie_writel_rc(pp, PCIE_ATU_ENABLE, PCIE_ATU_CR2);
+ /*
+ * ensure that the ATU enable has been happaned before accessing
+ * pci configuration/io spaces through dw_pcie_cfg_[read|write].
+ */
+ smp_wmb();
}
static struct irq_chip dw_msi_irq_chip = {
--
1.7.9.5
From: Stanimir Varbanov <[email protected]>
Document Qualcomm PCIe driver devicetree bindings.
Signed-off-by: Stanimir Varbanov <[email protected]>
Signed-off-by: Stanimir Varbanov <[email protected]>
---
.../devicetree/bindings/pci/qcom,pcie.txt | 231 ++++++++++++++++++++
1 file changed, 231 insertions(+)
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 000000000000..d7640d45fa31
--- /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", "snps,dw-pcie";
+ 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 238 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", "snps,dw-pcie";
+ 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.9.5
From: Stanimir Varbanov <[email protected]>
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 <[email protected]>
Signed-off-by: Stanimir Varbanov <[email protected]>
---
MAINTAINERS | 7 +
drivers/pci/host/Kconfig | 10 +
drivers/pci/host/Makefile | 1 +
drivers/pci/host/pcie-qcom.c | 623 ++++++++++++++++++++++++++++++++++++++++++
4 files changed, 641 insertions(+)
create mode 100644 drivers/pci/host/pcie-qcom.c
diff --git a/MAINTAINERS b/MAINTAINERS
index b16bffabe70a..878d9068fe75 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8231,6 +8231,13 @@ S: Maintained
F: Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
F: drivers/pci/host/pcie-hisi.c
+PCIE DRIVER FOR QUALCOMM MSM
+M: Stanimir Varbanov <[email protected]>
+L: [email protected]
+L: [email protected]
+S: Maintained
+F: drivers/pci/host/*qcom*
+
PCMCIA SUBSYSTEM
P: Linux PCMCIA Team
L: [email protected]
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index f131ba947dc6..64d33ba3e336 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -172,4 +172,14 @@ config PCI_HISI
help
Say Y here if you want PCIe controller support on HiSilicon HIP05 SoC
+config PCIE_QCOM
+ bool "Qualcomm PCIe controller"
+ depends on ARCH_QCOM && OF || 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 9d4d3c6924a1..e523c171febf 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -20,3 +20,4 @@ obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
obj-$(CONFIG_PCIE_ALTERA) += pcie-altera.o
obj-$(CONFIG_PCIE_ALTERA_MSI) += pcie-altera-msi.o
obj-$(CONFIG_PCI_HISI) += pcie-hisi.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 000000000000..5f1957988503
--- /dev/null
+++ b/drivers/pci/host/pcie-qcom.c
@@ -0,0 +1,623 @@
+/*
+ * Copyright (c) 2014-2015, The Linux Foundation. All rights reserved.
+ * Copyright 2015 Linaro Limited.
+ *
+ * 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/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.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_CTRL_LT_ENABLE BIT(0)
+
+#define PCIE20_ELBI_SYS_STTS 0x08
+#define XMLH_LINK_UP BIT(10)
+
+#define PCIE20_CAP 0x70
+#define PCIE20_CAP_LINKCTRLSTATUS (PCIE20_CAP + 0x10)
+#define PCIE20_CAP_LINKCTRLSTATUS_LINK_UP BIT(29)
+
+#define PERST_DELAY_MS 1
+
+#define LINKUP_DELAY_US 5000
+#define LINKUP_TIMEOUT_US 100000
+
+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 qcom_pcie_resources {
+ struct qcom_pcie_resources_v0 v0;
+ struct qcom_pcie_resources_v1 v1;
+};
+
+struct qcom_pcie;
+
+struct qcom_pcie_ops {
+ int (*get_resources)(struct qcom_pcie *pcie);
+ int (*init)(struct qcom_pcie *pcie);
+ void (*deinit)(struct qcom_pcie *pcie);
+};
+
+struct qcom_pcie {
+ struct pcie_port pp;
+ struct device *dev;
+ union qcom_pcie_resources res;
+ void __iomem *parf;
+ void __iomem *dbi;
+ void __iomem *elbi;
+ struct phy *phy;
+ struct gpio_desc *reset;
+ struct qcom_pcie_ops *ops;
+};
+
+#define to_qcom_pcie(x) container_of(x, struct qcom_pcie, pp)
+
+static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
+{
+ gpiod_set_value(pcie->reset, 1);
+ msleep(PERST_DELAY_MS);
+}
+
+static void qcom_ep_reset_deassert(struct qcom_pcie *pcie)
+{
+ gpiod_set_value(pcie->reset, 0);
+ msleep(PERST_DELAY_MS);
+}
+
+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_enable_link_training(struct qcom_pcie *pcie)
+{
+ struct device *dev = pcie->dev;
+ u32 val;
+ int ret;
+
+ /* enable link training */
+ val = readl(pcie->elbi + PCIE20_ELBI_SYS_CTRL);
+ val |= PCIE20_ELBI_SYS_CTRL_LT_ENABLE;
+ writel(val, pcie->elbi + PCIE20_ELBI_SYS_CTRL);
+
+ /* wait for up to 100ms for the link to come up */
+ ret = readl_poll_timeout(pcie->elbi + PCIE20_ELBI_SYS_STTS, val,
+ val & XMLH_LINK_UP, LINKUP_DELAY_US,
+ LINKUP_TIMEOUT_US);
+
+ if (ret < 0 || !dw_pcie_link_up(&pcie->pp)) {
+ dev_err(dev, "link initialization failed\n");
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+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))
+ return PTR_ERR(res->aux);
+
+ 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 void qcom_pcie_deinit_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 int qcom_pcie_init_v0(struct qcom_pcie *pcie)
+{
+ struct qcom_pcie_resources_v0 *res = &pcie->res.v0;
+ struct device *dev = pcie->dev;
+ u32 val;
+ 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 = reset_control_assert(res->ahb_reset);
+ if (ret) {
+ dev_err(dev, "cannot assert ahb reset\n");
+ goto err_assert_ahb;
+ }
+
+ ret = clk_prepare_enable(res->iface_clk);
+ if (ret) {
+ dev_err(dev, "cannot prepare/enable iface clock\n");
+ goto err_assert_ahb;
+ }
+
+ ret = clk_prepare_enable(res->phy_clk);
+ if (ret) {
+ dev_err(dev, "cannot prepare/enable phy clock\n");
+ goto err_clk_phy;
+ }
+
+ ret = clk_prepare_enable(res->core_clk);
+ if (ret) {
+ dev_err(dev, "cannot prepare/enable core clock\n");
+ goto err_clk_core;
+ }
+
+ ret = reset_control_deassert(res->ahb_reset);
+ if (ret) {
+ dev_err(dev, "cannot deassert ahb reset\n");
+ goto err_deassert_ahb;
+ }
+
+ /* enable PCIe clocks and resets */
+ val = readl(pcie->parf + PCIE20_PARF_PHY_CTRL);
+ val &= ~BIT(0);
+ writel(val, pcie->parf + PCIE20_PARF_PHY_CTRL);
+
+ /* enable external reference clock */
+ val = readl(pcie->parf + PCIE20_PARF_PHY_REFCLK);
+ val |= BIT(16);
+ writel(val, pcie->parf + PCIE20_PARF_PHY_REFCLK);
+
+ ret = reset_control_deassert(res->phy_reset);
+ if (ret) {
+ dev_err(dev, "cannot deassert phy reset\n");
+ return ret;
+ }
+
+ ret = reset_control_deassert(res->pci_reset);
+ if (ret) {
+ dev_err(dev, "cannot deassert pci reset\n");
+ return ret;
+ }
+
+ ret = reset_control_deassert(res->por_reset);
+ if (ret) {
+ dev_err(dev, "cannot deassert por reset\n");
+ return ret;
+ }
+
+ ret = reset_control_deassert(res->axi_reset);
+ if (ret) {
+ dev_err(dev, "cannot deassert axi reset\n");
+ return ret;
+ }
+
+ /* wait for clock acquisition */
+ usleep_range(1000, 1500);
+
+ return 0;
+
+err_deassert_ahb:
+ clk_disable_unprepare(res->core_clk);
+err_clk_core:
+ clk_disable_unprepare(res->phy_clk);
+err_clk_phy:
+ clk_disable_unprepare(res->iface_clk);
+err_assert_ahb:
+ regulator_disable(res->vdda_phy);
+err_vdda_phy:
+ regulator_disable(res->vdda_refclk);
+err_refclk:
+ regulator_disable(res->vdda);
+
+ return ret;
+}
+
+static void qcom_pcie_deinit_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_init_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;
+ }
+
+
+ /* change DBI base address */
+ writel(0, pcie->parf + PCIE20_PARF_DBI_BASE_ADDR);
+
+ if (IS_ENABLED(CONFIG_PCI_MSI)) {
+ u32 val = readl(pcie->parf + PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT);
+
+ val |= BIT(31);
+ writel(val, pcie->parf + PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT);
+ }
+
+ 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_link_up(struct pcie_port *pp)
+{
+ struct qcom_pcie *pcie = to_qcom_pcie(pp);
+ u32 val = readl(pcie->dbi + PCIE20_CAP_LINKCTRLSTATUS);
+
+ return !!(val & PCIE20_CAP_LINKCTRLSTATUS_LINK_UP);
+}
+
+static void qcom_pcie_host_init(struct pcie_port *pp)
+{
+ struct qcom_pcie *pcie = to_qcom_pcie(pp);
+ int ret;
+
+ qcom_ep_reset_assert(pcie);
+
+ ret = pcie->ops->init(pcie);
+ if (ret)
+ goto err_deinit;
+
+ ret = phy_power_on(pcie->phy);
+ if (ret)
+ goto err_deinit;
+
+ 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(pcie);
+ if (ret)
+ goto err;
+
+ return;
+err:
+ qcom_ep_reset_assert(pcie);
+ phy_power_off(pcie->phy);
+err_deinit:
+ pcie->ops->deinit(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 &= 0xff; /* keep revision id */
+ *val |= PCI_CLASS_BRIDGE_PCI << 16;
+ return PCIBIOS_SUCCESSFUL;
+ }
+
+ return dw_pcie_cfg_read(pp->dbi_base + where, size, val);
+}
+
+static struct pcie_host_ops qcom_pcie_dw_ops = {
+ .link_up = qcom_pcie_link_up,
+ .host_init = qcom_pcie_host_init,
+ .rd_own_conf = qcom_pcie_rd_own_conf,
+};
+
+static const struct qcom_pcie_ops ops_v0 = {
+ .get_resources = qcom_pcie_get_resources_v0,
+ .init = qcom_pcie_init_v0,
+ .deinit = qcom_pcie_deinit_v0,
+};
+
+static const struct qcom_pcie_ops ops_v1 = {
+ .get_resources = qcom_pcie_get_resources_v1,
+ .init = qcom_pcie_init_v1,
+ .deinit = qcom_pcie_deinit_v1,
+};
+
+static int qcom_pcie_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+ struct qcom_pcie *pcie;
+ struct pcie_port *pp;
+ int ret;
+
+ pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
+ if (!pcie)
+ return -ENOMEM;
+
+ pcie->ops = (struct qcom_pcie_ops *)of_device_get_match_data(dev);
+ pcie->dev = dev;
+
+ pcie->reset = devm_gpiod_get_optional(dev, "perst", GPIOD_OUT_LOW);
+ if (IS_ERR(pcie->reset))
+ 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);
+
+ ret = pcie->ops->get_resources(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_dw_ops;
+
+ if (IS_ENABLED(CONFIG_PCI_MSI)) {
+ pp->msi_irq = platform_get_irq_byname(pdev, "msi");
+ if (pp->msi_irq < 0)
+ 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 = phy_init(pcie->phy);
+ if (ret)
+ 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);
+ pcie->ops->deinit(pcie);
+
+ return 0;
+}
+
+static const struct of_device_id qcom_pcie_match[] = {
+ { .compatible = "qcom,pcie-v0", .data = &ops_v0 },
+ { .compatible = "qcom,pcie-v1", .data = &ops_v1 },
+ { }
+};
+MODULE_DEVICE_TABLE(of, qcom_pcie_match);
+
+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 <[email protected]>");
+MODULE_DESCRIPTION("Qualcomm PCIe root complex driver");
+MODULE_LICENSE("GPL v2");
--
1.7.9.5
Add the pcie dt node so that it can probe and used.
Signed-off-by: Stanimir Varbanov <[email protected]>
---
arch/arm/boot/dts/qcom-apq8064.dtsi | 36 +++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
index a4c1762b53ea..bb7181e8ef2f 100644
--- a/arch/arm/boot/dts/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
@@ -659,5 +659,41 @@
compatible = "qcom,tcsr-apq8064", "syscon";
reg = <0x1a400000 0x100>;
};
+
+ pcie: pci@1b500000 {
+ compatible = "qcom,pcie-v0", "snps,dw-pcie";
+ 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 0 0x08000000 0 0x07e00000>; /* memory */
+ interrupts = <GIC_SPI 238 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_REF_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";
+ status = "disabled";
+ };
};
};
--
1.7.9.5
Enable pcie dt node and fill pcie dt node with regulator, pinctrl
and reset gpio, to use the pcie on the ifc6410 board.
Signed-off-by: Stanimir Varbanov <[email protected]>
---
arch/arm/boot/dts/qcom-apq8064-ifc6410.dts | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
index 11ac608b6d50..bdee36a9b418 100644
--- a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
+++ b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
@@ -47,6 +47,18 @@
bias-disable;
};
};
+
+ pcie0_pins: pcie0_pinmux {
+ mux {
+ pins = "gpio27";
+ function = "gpio";
+ };
+ conf {
+ pins = "gpio27";
+ drive-strength = <12>;
+ bias-disable;
+ };
+ };
};
rpm@108000 {
@@ -123,6 +135,10 @@
pm8921_lvs1: lvs1 {
bias-pull-down;
};
+
+ pm8921_lvs6: lvs6 {
+ bias-pull-down;
+ };
};
};
@@ -231,6 +247,16 @@
status = "okay";
};
+ pci@1b500000 {
+ status = "ok";
+ vdda-supply = <&pm8921_s3>;
+ vdda_phy-supply = <&pm8921_lvs6>;
+ vdda_refclk-supply = <&ext_3p3v>;
+ pinctrl-0 = <&pcie0_pins>;
+ pinctrl-names = "default";
+ perst-gpio = <&tlmm_pinmux 27 GPIO_ACTIVE_LOW>;
+ };
+
qcom,ssbi@500000 {
pmic@0 {
gpio@150 {
--
1.7.9.5
On Monday 23 November 2015 11:28:58 Stanimir Varbanov wrote:
> The io_base is used to keep the cpu physical address parsed
> from ranges dt property. After issue pci_remap_iospace the
> io_base has been assigned with io->start, which is not correct
> cause io->start is a PCI bus address.
>
> Signed-off-by: Stanimir Varbanov <[email protected]>
> ---
> drivers/pci/host/pcie-designware.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index 540f077c37ea..02a7452bdf23 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -440,7 +440,6 @@ int dw_pcie_host_init(struct pcie_port *pp)
> ret, pp->io);
> continue;
> }
> - pp->io_base = pp->io->start;
> break;
> case IORESOURCE_MEM:
> pp->mem = win->res;
I was surprised to see such an obvious bug here, as we had spent a lot of
time trying to get it right. However, it broke only recently and it's
worth mentioning what commit did it, so
Fixes: 0021d22b73d6 ("PCI: designware: Use of_pci_get_host_bridge_resources() to parse DT")
Reviewed-by: Arnd Bergmann <[email protected]>
The bug is present in 4.4-rc1 and we should get your fix merged into 4.4
as well, while all the other patches in your series are presumably for 4.5.
Arnd
Hi Stanimir, Many Thanks for this fix
> -----Original Message-----
> From: [email protected] [mailto:linux-kernel-
> [email protected]] On Behalf Of Arnd Bergmann
> Sent: 23 November 2015 10:00
> To: Stanimir Varbanov
> Cc: [email protected]; [email protected]; linux-arm-
> [email protected]; [email protected]; linux-
> [email protected]; Bjorn Helgaas; Srinivas Kandagatla; Rob Herring; Rob
> Herring; Mark Rutland; Pawel Moll; Ian Campbell; Jingoo Han; Pratyush Anand;
> Bjorn Andersson
> Subject: Re: [PATCH v3 1/6] PCI: designware: remove wrong io_base assignment
>
> On Monday 23 November 2015 11:28:58 Stanimir Varbanov wrote:
> > The io_base is used to keep the cpu physical address parsed
> > from ranges dt property. After issue pci_remap_iospace the
> > io_base has been assigned with io->start, which is not correct
> > cause io->start is a PCI bus address.
> >
> > Signed-off-by: Stanimir Varbanov <[email protected]>
> > ---
> > drivers/pci/host/pcie-designware.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-
> designware.c
> > index 540f077c37ea..02a7452bdf23 100644
> > --- a/drivers/pci/host/pcie-designware.c
> > +++ b/drivers/pci/host/pcie-designware.c
> > @@ -440,7 +440,6 @@ int dw_pcie_host_init(struct pcie_port *pp)
> > ret, pp->io);
> > continue;
> > }
> > - pp->io_base = pp->io->start;
> > break;
> > case IORESOURCE_MEM:
> > pp->mem = win->res;
>
> I was surprised to see such an obvious bug here, as we had spent a lot of
> time trying to get it right. However, it broke only recently and it's
> worth mentioning what commit did it, so
Looking at
"[PATCH v12 0/8] PCI: hisi: Add PCIe host support for HiSilicon SoC Hip05":
actually the bug came in as patch 3/6 of v11 patchset split
[...]
Change from v11:
- Split 3/6 in v11 to 3/8, 4/8, 5/8 in v12.
[...]
This was not present in v11...sorry about this.
Gab
>
> Fixes: 0021d22b73d6 ("PCI: designware: Use of_pci_get_host_bridge_resources()
> to parse DT")
> Reviewed-by: Arnd Bergmann <[email protected]>
>
> The bug is present in 4.4-rc1 and we should get your fix merged into 4.4
> as well, while all the other patches in your series are presumably for 4.5.
>
> Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
Hi Stanimir,
[auto build test ERROR on: v4.4-rc2]
[also build test ERROR on: next-20151123]
url: https://github.com/0day-ci/linux/commits/Stanimir-Varbanov/Qualcomm-PCIe-driver-and-designware-fixes/20151123-173312
config: i386-allmodconfig (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers/pci/host/pcie-designware.c: In function 'dw_pcie_host_init':
>> drivers/pci/host/pcie-designware.c:559:7: error: implicit declaration of function 'pci_has_flag' [-Werror=implicit-function-declaration]
if (!pci_has_flag(PCI_PROBE_ONLY)) {
^
>> drivers/pci/host/pcie-designware.c:559:20: error: 'PCI_PROBE_ONLY' undeclared (first use in this function)
if (!pci_has_flag(PCI_PROBE_ONLY)) {
^
drivers/pci/host/pcie-designware.c:559:20: note: each undeclared identifier is reported only once for each function it appears in
cc1: some warnings being treated as errors
vim +/pci_has_flag +559 drivers/pci/host/pcie-designware.c
cbce7900 Zhou Wang 2015-10-29 543 &dw_pcie_msi_chip);
0815f957 Yijing Wang 2014-11-11 544 dw_pcie_msi_chip.dev = pp->dev;
cbce7900 Zhou Wang 2015-10-29 545 } else
cbce7900 Zhou Wang 2015-10-29 546 bus = pci_scan_root_bus(pp->dev, pp->root_bus_nr, &dw_pcie_ops,
cbce7900 Zhou Wang 2015-10-29 547 pp, &res);
cbce7900 Zhou Wang 2015-10-29 548 if (!bus)
cbce7900 Zhou Wang 2015-10-29 549 return -ENOMEM;
cbce7900 Zhou Wang 2015-10-29 550
cbce7900 Zhou Wang 2015-10-29 551 if (pp->ops->scan_bus)
cbce7900 Zhou Wang 2015-10-29 552 pp->ops->scan_bus(pp);
cbce7900 Zhou Wang 2015-10-29 553
cbce7900 Zhou Wang 2015-10-29 554 #ifdef CONFIG_ARM
cbce7900 Zhou Wang 2015-10-29 555 /* support old dtbs that incorrectly describe IRQs */
cbce7900 Zhou Wang 2015-10-29 556 pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
0815f957 Yijing Wang 2014-11-11 557 #endif
0815f957 Yijing Wang 2014-11-11 558
cbce7900 Zhou Wang 2015-10-29 @559 if (!pci_has_flag(PCI_PROBE_ONLY)) {
cbce7900 Zhou Wang 2015-10-29 560 pci_bus_size_bridges(bus);
cbce7900 Zhou Wang 2015-10-29 561 pci_bus_assign_resources(bus);
4b1ced84 Jingoo Han 2013-07-31 562
cbce7900 Zhou Wang 2015-10-29 563 list_for_each_entry(child, &bus->children, node)
cbce7900 Zhou Wang 2015-10-29 564 pcie_bus_configure_settings(child);
cbce7900 Zhou Wang 2015-10-29 565 }
4b1ced84 Jingoo Han 2013-07-31 566
cbce7900 Zhou Wang 2015-10-29 567 pci_bus_add_devices(bus);
:::::: The code at line 559 was first introduced by commit
:::::: cbce7900598c26a12652f8ca9c41c5b29034c38d PCI: designware: Make driver arch-agnostic
:::::: TO: Zhou Wang <[email protected]>
:::::: CC: Bjorn Helgaas <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Mon, Nov 23, 2015 at 11:28:59AM +0200, Stanimir Varbanov wrote:
> Add 'write memory' barrier after enable region in PCIE_ATU_CR2
> register. The barrier is needed to ensure that the region enable
> request has been reached it's destination at time when we
> read/write to PCI configuration space.
>
> Without this barrier PCI device enumeration during kernel boot
> is not reliable, and reading configuration space for particular
> PCI device on the bus returns zero aka no device.
>
> Signed-off-by: Stanimir Varbanov <[email protected]>
> ---
> drivers/pci/host/pcie-designware.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index 02a7452bdf23..e15a2ae1583f 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -164,6 +164,11 @@ static void dw_pcie_prog_outbound_atu(struct pcie_port *pp, int index,
> dw_pcie_writel_rc(pp, upper_32_bits(pci_addr), PCIE_ATU_UPPER_TARGET);
> dw_pcie_writel_rc(pp, type, PCIE_ATU_CR1);
> dw_pcie_writel_rc(pp, PCIE_ATU_ENABLE, PCIE_ATU_CR2);
> + /*
> + * ensure that the ATU enable has been happaned before accessing
> + * pci configuration/io spaces through dw_pcie_cfg_[read|write].
> + */
> + smp_wmb();
So, why is this a SMP barrier?
--
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
On 11/23/2015 01:27 PM, Russell King - ARM Linux wrote:
> On Mon, Nov 23, 2015 at 11:28:59AM +0200, Stanimir Varbanov wrote:
>> Add 'write memory' barrier after enable region in PCIE_ATU_CR2
>> register. The barrier is needed to ensure that the region enable
>> request has been reached it's destination at time when we
>> read/write to PCI configuration space.
>>
>> Without this barrier PCI device enumeration during kernel boot
>> is not reliable, and reading configuration space for particular
>> PCI device on the bus returns zero aka no device.
>>
>> Signed-off-by: Stanimir Varbanov <[email protected]>
>> ---
>> drivers/pci/host/pcie-designware.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
>> index 02a7452bdf23..e15a2ae1583f 100644
>> --- a/drivers/pci/host/pcie-designware.c
>> +++ b/drivers/pci/host/pcie-designware.c
>> @@ -164,6 +164,11 @@ static void dw_pcie_prog_outbound_atu(struct pcie_port *pp, int index,
>> dw_pcie_writel_rc(pp, upper_32_bits(pci_addr), PCIE_ATU_UPPER_TARGET);
>> dw_pcie_writel_rc(pp, type, PCIE_ATU_CR1);
>> dw_pcie_writel_rc(pp, PCIE_ATU_ENABLE, PCIE_ATU_CR2);
>> + /*
>> + * ensure that the ATU enable has been happaned before accessing
>> + * pci configuration/io spaces through dw_pcie_cfg_[read|write].
>> + */
>> + smp_wmb();
>
> So, why is this a SMP barrier?
>
I wrongly assumed that smp_wmb will come down to wmb(), I have no excuse.
But despite my ignorant, as I noted in cover letter I want this patch to
be treated as an RFC (maybe I had to add it in the subject).
Do we really need a memory barrier after enabling ATU in PCI_ATU_CR2
register and before first access to pci configuration space using
read[lwb] (see dw_pcie_rd_other_conf)? Or I have made totally wrong
assumption.
--
regards,
Stan
On 11/23/2015 12:27 PM, Gabriele Paoloni wrote:
> Hi Stanimir, Many Thanks for this fix
>
>> -----Original Message-----
>> From: [email protected] [mailto:linux-kernel-
>> [email protected]] On Behalf Of Arnd Bergmann
>> Sent: 23 November 2015 10:00
>> To: Stanimir Varbanov
>> Cc: [email protected]; [email protected]; linux-arm-
>> [email protected]; [email protected]; linux-
>> [email protected]; Bjorn Helgaas; Srinivas Kandagatla; Rob Herring; Rob
>> Herring; Mark Rutland; Pawel Moll; Ian Campbell; Jingoo Han; Pratyush Anand;
>> Bjorn Andersson
>> Subject: Re: [PATCH v3 1/6] PCI: designware: remove wrong io_base assignment
>>
>> On Monday 23 November 2015 11:28:58 Stanimir Varbanov wrote:
>>> The io_base is used to keep the cpu physical address parsed
>>> from ranges dt property. After issue pci_remap_iospace the
>>> io_base has been assigned with io->start, which is not correct
>>> cause io->start is a PCI bus address.
>>>
>>> Signed-off-by: Stanimir Varbanov <[email protected]>
>>> ---
>>> drivers/pci/host/pcie-designware.c | 1 -
>>> 1 file changed, 1 deletion(-)
>>>
>>> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-
>> designware.c
>>> index 540f077c37ea..02a7452bdf23 100644
>>> --- a/drivers/pci/host/pcie-designware.c
>>> +++ b/drivers/pci/host/pcie-designware.c
>>> @@ -440,7 +440,6 @@ int dw_pcie_host_init(struct pcie_port *pp)
>>> ret, pp->io);
>>> continue;
>>> }
>>> - pp->io_base = pp->io->start;
>>> break;
>>> case IORESOURCE_MEM:
>>> pp->mem = win->res;
>>
>> I was surprised to see such an obvious bug here, as we had spent a lot of
>> time trying to get it right. However, it broke only recently and it's
>> worth mentioning what commit did it, so
>
> Looking at
> "[PATCH v12 0/8] PCI: hisi: Add PCIe host support for HiSilicon SoC Hip05":
>
> actually the bug came in as patch 3/6 of v11 patchset split
>
> [...]
>
> Change from v11:
> - Split 3/6 in v11 to 3/8, 4/8, 5/8 in v12.
>
> [...]
>
> This was not present in v11...sorry about this.
>
> Gab
>
>>
>> Fixes: 0021d22b73d6 ("PCI: designware: Use of_pci_get_host_bridge_resources()
>> to parse DT")
>> Reviewed-by: Arnd Bergmann <[email protected]>
I think the bug is introduced in:
cbce7900598c ("PCI: designware: Make driver arch-agnostic")
cause the io_base is correctly calculated in 0021d22b73d6, do you agree?
--
regards,
Stan
On Monday 23 November 2015 18:23:47 Stanimir Varbanov wrote:
> >>
> >> Fixes: 0021d22b73d6 ("PCI: designware: Use of_pci_get_host_bridge_resources()
> >> to parse DT")
> >> Reviewed-by: Arnd Bergmann <[email protected]>
>
> I think the bug is introduced in:
>
> cbce7900598c ("PCI: designware: Make driver arch-agnostic")
>
> cause the io_base is correctly calculated in 0021d22b73d6, do you agree?
I think cbce7900598c just slightly changes the io_base value, but
it's still referring to a bus address, not a cpu address, both before
and after the patch.
0021d22b73d6 hower seems to remove the correct 'pp->io_base = range.cpu_addr;'
and replaces it with 'pp->io_base = pp->io->start;', so it's now in
the wrong address space.
Arnd
On Mon 23 Nov 01:29 PST 2015, Stanimir Varbanov wrote:
> From: Stanimir Varbanov <[email protected]>
>
> Document Qualcomm PCIe driver devicetree bindings.
>
> Signed-off-by: Stanimir Varbanov <[email protected]>
> Signed-off-by: Stanimir Varbanov <[email protected]>
> ---
> .../devicetree/bindings/pci/qcom,pcie.txt | 231 ++++++++++++++++++++
> 1 file changed, 231 insertions(+)
> 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 000000000000..d7640d45fa31
> --- /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
Do you know if we have the same v1 of this block in 8994?
[..]
> +- 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
You have white spaces among your tabs here.
[..]
> +- <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
Exploding these into 3 different property descriptions would make it
easier to read, and you can say "required for v0" for the latter
two and simply "required" on the vdda.
[..]
> +- <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
This property should be pluralized, i.e. it's <name>-gpios.
Are these identifiers coming from some data sheet? Or could we simply
name them "reset" and "wakeup"?
> +
> +- pinctrl-0:
> + Usage: required
> + Value type: <phandle>
> + Definition: List of phandles pointing at a pin(s) configuration
This is not required and as it's a property applicable to all devices we
normally don't mention them.
> +
> +- pinctrl-names
> + Usage: required
> + Value type: <stringlist>
> + Definition: List of names of pinctrl-0 state
> +
dito
Regards,
Bjorn
On Mon, Nov 23, 2015 at 11:29:00AM +0200, Stanimir Varbanov wrote:
> From: Stanimir Varbanov <[email protected]>
>
> Document Qualcomm PCIe driver devicetree bindings.
>
> Signed-off-by: Stanimir Varbanov <[email protected]>
> Signed-off-by: Stanimir Varbanov <[email protected]>
> ---
> .../devicetree/bindings/pci/qcom,pcie.txt | 231 ++++++++++++++++++++
> 1 file changed, 231 insertions(+)
> 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 000000000000..d7640d45fa31
> --- /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
Use chip part numbers, not versions.
Also, shouldn't you have the Designware compatible in addition.
[...]
> +
> +- <name>-gpio:
Use -gpios
> + 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
Bjorn, thanks for the comments!
On 11/23/2015 08:13 PM, Bjorn Andersson wrote:
> On Mon 23 Nov 01:29 PST 2015, Stanimir Varbanov wrote:
>
>> From: Stanimir Varbanov <[email protected]>
>>
>> Document Qualcomm PCIe driver devicetree bindings.
>>
>> Signed-off-by: Stanimir Varbanov <[email protected]>
>> Signed-off-by: Stanimir Varbanov <[email protected]>
>> ---
>> .../devicetree/bindings/pci/qcom,pcie.txt | 231 ++++++++++++++++++++
>> 1 file changed, 231 insertions(+)
>> 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 000000000000..d7640d45fa31
>> --- /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
>
> Do you know if we have the same v1 of this block in 8994?
I have no idea, but looking in caf msm-3.18 it should be possible to
reuse the code for v1.
>
> [..]
>> +- 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
>
> You have white spaces among your tabs here.
Ops, I forgot to remove the white spaces.
>
> [..]
>> +- <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
>
> Exploding these into 3 different property descriptions would make it
> easier to read, and you can say "required for v0" for the latter
> two and simply "required" on the vdda.
yes, that is a good idea.
>
> [..]
>> +- <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
>
> This property should be pluralized, i.e. it's <name>-gpios.
I hope you mean :
- "perst-gpios" PCIe endpoint reset signal line
- "pewake-gpios" PCIe endpoint wake signal line
>
> Are these identifiers coming from some data sheet? Or could we simply
> name them "reset" and "wakeup"?
In the pcie express card electromechanical specification we have signal
names PERST# and WAKE#, so I'd like to keep as they are in the
specification, thus the wake# is wrong and I will rename it to wake-gpios.
>
>> +
>> +- pinctrl-0:
>> + Usage: required
>> + Value type: <phandle>
>> + Definition: List of phandles pointing at a pin(s) configuration
>
> This is not required and as it's a property applicable to all devices we
> normally don't mention them.
agreed.
>
>> +
>> +- pinctrl-names
>> + Usage: required
>> + Value type: <stringlist>
>> + Definition: List of names of pinctrl-0 state
>> +
>
> dito
>
> Regards,
> Bjorn
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
regards,
Stan
Thanks for the comments!
On 11/24/2015 01:17 AM, Rob Herring wrote:
> On Mon, Nov 23, 2015 at 11:29:00AM +0200, Stanimir Varbanov wrote:
>> From: Stanimir Varbanov <[email protected]>
>>
>> Document Qualcomm PCIe driver devicetree bindings.
>>
>> Signed-off-by: Stanimir Varbanov <[email protected]>
>> Signed-off-by: Stanimir Varbanov <[email protected]>
>> ---
>> .../devicetree/bindings/pci/qcom,pcie.txt | 231 ++++++++++++++++++++
>> 1 file changed, 231 insertions(+)
>> 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 000000000000..d7640d45fa31
>> --- /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
>
> Use chip part numbers, not versions.
Sure.
>
> Also, shouldn't you have the Designware compatible in addition.
>
> [...]
>
>> +
>> +- <name>-gpio:
>
> Use -gpios
Ok I will.
--
regards,
Stan
On 11/23/2015 06:40 PM, Arnd Bergmann wrote:
> On Monday 23 November 2015 18:23:47 Stanimir Varbanov wrote:
>>>>
>>>> Fixes: 0021d22b73d6 ("PCI: designware: Use of_pci_get_host_bridge_resources()
>>>> to parse DT")
>>>> Reviewed-by: Arnd Bergmann <[email protected]>
>>
>> I think the bug is introduced in:
>>
>> cbce7900598c ("PCI: designware: Make driver arch-agnostic")
>>
>> cause the io_base is correctly calculated in 0021d22b73d6, do you agree?
>
>
> I think cbce7900598c just slightly changes the io_base value, but
> it's still referring to a bus address, not a cpu address, both before
> and after the patch.
>
> 0021d22b73d6 hower seems to remove the correct 'pp->io_base = range.cpu_addr;'
> and replaces it with 'pp->io_base = pp->io->start;', so it's now in
> the wrong address space.
>
> Arnd
>
OK, I will resend this as separate patch, and will drop it from this series.
--
regards,
Stan