All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/5] Qualcomm PCIe driver and designware fixes
@ 2015-12-18 12:38 ` Stanimir Varbanov
  0 siblings, 0 replies; 38+ messages in thread
From: Stanimir Varbanov @ 2015-12-18 12:38 UTC (permalink / raw)
  To: linux-arm-msm, linux-kernel, linux-arm-kernel, devicetree,
	linux-pci, Bjorn Helgaas
  Cc: Srinivas Kandagatla, Russell King, Rob Herring, Rob Herring,
	Mark Rutland, Pawel Moll, Ian Campbell, Arnd Bergmann,
	Jingoo Han, Pratyush Anand, Bjorn Andersson, Stanimir Varbanov

Hi,

I'm sending v5 with following changes:
- in 1/5 - replace wmb() call with PCIE_ATU_CR2 register read.
- in 3/5 - addressed comments from Born Helgaas about usage of a
standard PCI capabilities register names and rename link training
function to the way that other Designware based drivers use.
- in 5/5 - addressed a comment from Bjorn Andersson about regulator
label duplication.

Comments are welcome!

The previous v4 of the patch set can be found at [1].

regards,
Stan

[1] https://lkml.org/lkml/2015/12/3/370

Stanimir Varbanov (5):
  PCI: designware: ensure ATU is enabled before IO/conf space accesses
  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          |  233 ++++++++
 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                 |    7 +
 drivers/pci/host/pcie-qcom.c                       |  617 ++++++++++++++++++++
 8 files changed, 937 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/qcom,pcie.txt
 create mode 100644 drivers/pci/host/pcie-qcom.c

-- 
1.7.9.5

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

* [PATCH v5 0/5] Qualcomm PCIe driver and designware fixes
@ 2015-12-18 12:38 ` Stanimir Varbanov
  0 siblings, 0 replies; 38+ messages in thread
From: Stanimir Varbanov @ 2015-12-18 12:38 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

I'm sending v5 with following changes:
- in 1/5 - replace wmb() call with PCIE_ATU_CR2 register read.
- in 3/5 - addressed comments from Born Helgaas about usage of a
standard PCI capabilities register names and rename link training
function to the way that other Designware based drivers use.
- in 5/5 - addressed a comment from Bjorn Andersson about regulator
label duplication.

Comments are welcome!

The previous v4 of the patch set can be found at [1].

regards,
Stan

[1] https://lkml.org/lkml/2015/12/3/370

Stanimir Varbanov (5):
  PCI: designware: ensure ATU is enabled before IO/conf space accesses
  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          |  233 ++++++++
 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                 |    7 +
 drivers/pci/host/pcie-qcom.c                       |  617 ++++++++++++++++++++
 8 files changed, 937 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/qcom,pcie.txt
 create mode 100644 drivers/pci/host/pcie-qcom.c

-- 
1.7.9.5

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

* [PATCH v5 1/5] PCI: designware: ensure ATU is enabled before IO/conf space accesses
  2015-12-18 12:38 ` Stanimir Varbanov
@ 2015-12-18 12:38   ` Stanimir Varbanov
  -1 siblings, 0 replies; 38+ messages in thread
From: Stanimir Varbanov @ 2015-12-18 12:38 UTC (permalink / raw)
  To: linux-arm-msm, linux-kernel, linux-arm-kernel, devicetree,
	linux-pci, Bjorn Helgaas
  Cc: Srinivas Kandagatla, Russell King, Rob Herring, Rob Herring,
	Mark Rutland, Pawel Moll, Ian Campbell, Arnd Bergmann,
	Jingoo Han, Pratyush Anand, Bjorn Andersson, Stanimir Varbanov

There is no guarantees that enabling ATU will hit the hardware
immediately, and subsequent accesses to configuration / IO spaces
are reliable. So fixing this by read back PCIE_ATU_CR2 register
just after writing.

Without such a fix the 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 <stanimir.varbanov@linaro.org>
---
 drivers/pci/host/pcie-designware.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 02a7452bdf23..7880de63895d 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -154,6 +154,8 @@ static int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
 static void dw_pcie_prog_outbound_atu(struct pcie_port *pp, int index,
 		int type, u64 cpu_addr, u64 pci_addr, u32 size)
 {
+	u32 val;
+
 	dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | index,
 			  PCIE_ATU_VIEWPORT);
 	dw_pcie_writel_rc(pp, lower_32_bits(cpu_addr), PCIE_ATU_LOWER_BASE);
@@ -164,6 +166,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].
+	 */
+	dw_pcie_readl_rc(pp, PCIE_ATU_CR2, &val);
 }
 
 static struct irq_chip dw_msi_irq_chip = {
-- 
1.7.9.5

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

* [PATCH v5 1/5] PCI: designware: ensure ATU is enabled before IO/conf space accesses
@ 2015-12-18 12:38   ` Stanimir Varbanov
  0 siblings, 0 replies; 38+ messages in thread
From: Stanimir Varbanov @ 2015-12-18 12:38 UTC (permalink / raw)
  To: linux-arm-kernel

There is no guarantees that enabling ATU will hit the hardware
immediately, and subsequent accesses to configuration / IO spaces
are reliable. So fixing this by read back PCIE_ATU_CR2 register
just after writing.

Without such a fix the 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 <stanimir.varbanov@linaro.org>
---
 drivers/pci/host/pcie-designware.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 02a7452bdf23..7880de63895d 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -154,6 +154,8 @@ static int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
 static void dw_pcie_prog_outbound_atu(struct pcie_port *pp, int index,
 		int type, u64 cpu_addr, u64 pci_addr, u32 size)
 {
+	u32 val;
+
 	dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | index,
 			  PCIE_ATU_VIEWPORT);
 	dw_pcie_writel_rc(pp, lower_32_bits(cpu_addr), PCIE_ATU_LOWER_BASE);
@@ -164,6 +166,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].
+	 */
+	dw_pcie_readl_rc(pp, PCIE_ATU_CR2, &val);
 }
 
 static struct irq_chip dw_msi_irq_chip = {
-- 
1.7.9.5

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

* [PATCH v5 2/5] DT: PCI: qcom: Document PCIe devicetree bindings
  2015-12-18 12:38 ` Stanimir Varbanov
@ 2015-12-18 12:38   ` Stanimir Varbanov
  -1 siblings, 0 replies; 38+ messages in thread
From: Stanimir Varbanov @ 2015-12-18 12:38 UTC (permalink / raw)
  To: linux-arm-msm, linux-kernel, linux-arm-kernel, devicetree,
	linux-pci, Bjorn Helgaas
  Cc: Srinivas Kandagatla, Russell King, Rob Herring, Rob Herring,
	Mark Rutland, Pawel Moll, Ian Campbell, Arnd Bergmann,
	Jingoo Han, Pratyush Anand, Bjorn Andersson, Stanimir Varbanov,
	Stanimir Varbanov

From: Stanimir Varbanov <svarbanov@mm-sol.com>

Document Qualcomm PCIe driver devicetree bindings.

Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
Acked-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/pci/qcom,pcie.txt          |  233 ++++++++++++++++++++
 1 file changed, 233 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..6d71ee2e335d
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
@@ -0,0 +1,233 @@
+* Qualcomm PCI express root complex
+
+- compatible:
+	Usage: required
+	Value type: <stringlist>
+	Definition: Value should contain
+			- "qcom,pcie-ipq8064" for ipq8064
+			- "qcom,pcie-apq8064" for apq8064
+			- "qcom,pcie-apq8084" 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
+			- "iface"	Configuration AHB clock
+
+- clock-names:
+	Usage: required for ipq/apq8064
+	Value type: <stringlist>
+	Definition: Should contain the following entries
+			- "core"	Clocks the pcie hw block
+			- "phy"		Clocks the pcie PHY block
+- clock-names:
+	Usage: required for apq8084
+	Value type: <stringlist>
+	Definition: Should contain the following entries
+			- "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 for ipq/apq8064
+	Value type: <stringlist>
+	Definition: Should contain the following entries
+			- "axi"  AXI reset
+			- "ahb"  AHB reset
+			- "por"  POR reset
+			- "pci"  PCI reset
+			- "phy"  PHY reset
+
+- reset-names:
+	Usage: required for apq8084
+	Value type: <stringlist>
+	Definition: Should contain the following entries
+			- "core" Core reset
+
+- power-domains:
+	Usage: required for apq8084
+	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
+
+- vdda-supply:
+	Usage: required
+	Value type: <phandle>
+	Definition: A phandle to the core analog power supply
+
+- vdda_phy-supply:
+	Usage: required for ipq/apq8064
+	Value type: <phandle>
+	Definition: A phandle to the analog power supply for PHY
+
+- vdda_refclk-supply:
+	Usage: required for ipq/apq8064
+	Value type: <phandle>
+	Definition: A phandle to the analog power supply for IC which generates
+		    reference clock
+
+- phys:
+	Usage: required for apq8084
+	Value type: <phandle>
+	Definition: List of phandle(s) as listed in phy-names property
+
+- phy-names:
+	Usage: required for apq8084
+	Value type: <stringlist>
+	Definition: Should contain "pciephy"
+
+- <name>-gpios:
+	Usage: optional
+	Value type: <prop-encoded-array>
+	Definition: List of phandle and gpio specifier pairs. Should contain
+			- "perst-gpios"	PCIe endpoint reset signal line
+			- "wake-gpios"	PCIe endpoint wake signal line
+
+* Example for ipq/apq8064
+	pcie@1b500000 {
+		compatible = "qcom,pcie-apq8064", "qcom,pcie-ipq8064", "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_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";
+		pinctrl-0 = <&pcie_pins_default>;
+		pinctrl-names = "default";
+	};
+
+* Example for apq8084
+	pcie0@fc520000 {
+		compatible = "qcom,pcie-apq8084", "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

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

* [PATCH v5 2/5] DT: PCI: qcom: Document PCIe devicetree bindings
@ 2015-12-18 12:38   ` Stanimir Varbanov
  0 siblings, 0 replies; 38+ messages in thread
From: Stanimir Varbanov @ 2015-12-18 12:38 UTC (permalink / raw)
  To: linux-arm-kernel

From: Stanimir Varbanov <svarbanov@mm-sol.com>

Document Qualcomm PCIe driver devicetree bindings.

Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
Acked-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/pci/qcom,pcie.txt          |  233 ++++++++++++++++++++
 1 file changed, 233 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..6d71ee2e335d
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
@@ -0,0 +1,233 @@
+* Qualcomm PCI express root complex
+
+- compatible:
+	Usage: required
+	Value type: <stringlist>
+	Definition: Value should contain
+			- "qcom,pcie-ipq8064" for ipq8064
+			- "qcom,pcie-apq8064" for apq8064
+			- "qcom,pcie-apq8084" 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
+			- "iface"	Configuration AHB clock
+
+- clock-names:
+	Usage: required for ipq/apq8064
+	Value type: <stringlist>
+	Definition: Should contain the following entries
+			- "core"	Clocks the pcie hw block
+			- "phy"		Clocks the pcie PHY block
+- clock-names:
+	Usage: required for apq8084
+	Value type: <stringlist>
+	Definition: Should contain the following entries
+			- "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 for ipq/apq8064
+	Value type: <stringlist>
+	Definition: Should contain the following entries
+			- "axi"  AXI reset
+			- "ahb"  AHB reset
+			- "por"  POR reset
+			- "pci"  PCI reset
+			- "phy"  PHY reset
+
+- reset-names:
+	Usage: required for apq8084
+	Value type: <stringlist>
+	Definition: Should contain the following entries
+			- "core" Core reset
+
+- power-domains:
+	Usage: required for apq8084
+	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
+
+- vdda-supply:
+	Usage: required
+	Value type: <phandle>
+	Definition: A phandle to the core analog power supply
+
+- vdda_phy-supply:
+	Usage: required for ipq/apq8064
+	Value type: <phandle>
+	Definition: A phandle to the analog power supply for PHY
+
+- vdda_refclk-supply:
+	Usage: required for ipq/apq8064
+	Value type: <phandle>
+	Definition: A phandle to the analog power supply for IC which generates
+		    reference clock
+
+- phys:
+	Usage: required for apq8084
+	Value type: <phandle>
+	Definition: List of phandle(s) as listed in phy-names property
+
+- phy-names:
+	Usage: required for apq8084
+	Value type: <stringlist>
+	Definition: Should contain "pciephy"
+
+- <name>-gpios:
+	Usage: optional
+	Value type: <prop-encoded-array>
+	Definition: List of phandle and gpio specifier pairs. Should contain
+			- "perst-gpios"	PCIe endpoint reset signal line
+			- "wake-gpios"	PCIe endpoint wake signal line
+
+* Example for ipq/apq8064
+	pcie at 1b500000 {
+		compatible = "qcom,pcie-apq8064", "qcom,pcie-ipq8064", "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_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";
+		pinctrl-0 = <&pcie_pins_default>;
+		pinctrl-names = "default";
+	};
+
+* Example for apq8084
+	pcie0 at fc520000 {
+		compatible = "qcom,pcie-apq8084", "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

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

* [PATCH v5 3/5] PCI: qcom: Add Qualcomm PCIe controller driver
  2015-12-18 12:38 ` Stanimir Varbanov
  (?)
@ 2015-12-18 12:38   ` Stanimir Varbanov
  -1 siblings, 0 replies; 38+ messages in thread
From: Stanimir Varbanov @ 2015-12-18 12:38 UTC (permalink / raw)
  To: linux-arm-msm, linux-kernel, linux-arm-kernel, devicetree,
	linux-pci, Bjorn Helgaas
  Cc: Mark Rutland, Russell King, Arnd Bergmann, Pawel Moll,
	Ian Campbell, Jingoo Han, Pratyush Anand, Stanimir Varbanov,
	Stanimir Varbanov, Rob Herring, Srinivas Kandagatla,
	Bjorn Andersson

From: Stanimir Varbanov <svarbanov@mm-sol.com>

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>
Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 MAINTAINERS                  |    7 +
 drivers/pci/host/Kconfig     |   10 +
 drivers/pci/host/Makefile    |    1 +
 drivers/pci/host/pcie-qcom.c |  617 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 635 insertions(+)
 create mode 100644 drivers/pci/host/pcie-qcom.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 38df53f828e1..7de5d2597e2b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8247,6 +8247,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 <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 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..36c81973467d
--- /dev/null
+++ b/drivers/pci/host/pcie-qcom.c
@@ -0,0 +1,617 @@
+/*
+ * 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_CAP				0x70
+
+#define PERST_DELAY_US				1000
+
+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);
+	usleep_range(PERST_DELAY_US, PERST_DELAY_US + 500);
+}
+
+static void qcom_ep_reset_deassert(struct qcom_pcie *pcie)
+{
+	gpiod_set_value(pcie->reset, 0);
+	usleep_range(PERST_DELAY_US, PERST_DELAY_US + 500);
+}
+
+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_establish_link(struct qcom_pcie *pcie)
+{
+	struct device *dev = pcie->dev;
+	unsigned int retries = 0;
+	u32 val;
+
+	if (dw_pcie_link_up(&pcie->pp))
+		return 0;
+
+	/* 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);
+
+	do {
+		if (dw_pcie_link_up(&pcie->pp))
+			return 0;
+		usleep_range(250, 1000);
+	} while (retries < 200);
+
+	dev_warn(dev, "phy link never came up\n");
+
+	return -ETIMEDOUT;
+}
+
+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);
+	u16 val = readw(pcie->dbi + PCIE20_CAP + PCI_EXP_LNKSTA);
+
+	return !!(val & PCI_EXP_LNKSTA_DLLLA);
+}
+
+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_establish_link(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-ipq8064", .data = &ops_v0 },
+	{ .compatible = "qcom,pcie-apq8064", .data = &ops_v0 },
+	{ .compatible = "qcom,pcie-apq8084", .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 <svarbanov@mm-sol.com>");
+MODULE_DESCRIPTION("Qualcomm PCIe root complex driver");
+MODULE_LICENSE("GPL v2");
-- 
1.7.9.5

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

* [PATCH v5 3/5] PCI: qcom: Add Qualcomm PCIe controller driver
@ 2015-12-18 12:38   ` Stanimir Varbanov
  0 siblings, 0 replies; 38+ messages in thread
From: Stanimir Varbanov @ 2015-12-18 12:38 UTC (permalink / raw)
  To: linux-arm-msm, linux-kernel, linux-arm-kernel, devicetree,
	linux-pci, Bjorn Helgaas
  Cc: Srinivas Kandagatla, Russell King, Rob Herring, Rob Herring,
	Mark Rutland, Pawel Moll, Ian Campbell, Arnd Bergmann,
	Jingoo Han, Pratyush Anand, Bjorn Andersson, Stanimir Varbanov,
	Stanimir Varbanov

From: Stanimir Varbanov <svarbanov@mm-sol.com>

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>
Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 MAINTAINERS                  |    7 +
 drivers/pci/host/Kconfig     |   10 +
 drivers/pci/host/Makefile    |    1 +
 drivers/pci/host/pcie-qcom.c |  617 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 635 insertions(+)
 create mode 100644 drivers/pci/host/pcie-qcom.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 38df53f828e1..7de5d2597e2b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8247,6 +8247,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 <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 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..36c81973467d
--- /dev/null
+++ b/drivers/pci/host/pcie-qcom.c
@@ -0,0 +1,617 @@
+/*
+ * 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_CAP				0x70
+
+#define PERST_DELAY_US				1000
+
+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);
+	usleep_range(PERST_DELAY_US, PERST_DELAY_US + 500);
+}
+
+static void qcom_ep_reset_deassert(struct qcom_pcie *pcie)
+{
+	gpiod_set_value(pcie->reset, 0);
+	usleep_range(PERST_DELAY_US, PERST_DELAY_US + 500);
+}
+
+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_establish_link(struct qcom_pcie *pcie)
+{
+	struct device *dev = pcie->dev;
+	unsigned int retries = 0;
+	u32 val;
+
+	if (dw_pcie_link_up(&pcie->pp))
+		return 0;
+
+	/* 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);
+
+	do {
+		if (dw_pcie_link_up(&pcie->pp))
+			return 0;
+		usleep_range(250, 1000);
+	} while (retries < 200);
+
+	dev_warn(dev, "phy link never came up\n");
+
+	return -ETIMEDOUT;
+}
+
+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);
+	u16 val = readw(pcie->dbi + PCIE20_CAP + PCI_EXP_LNKSTA);
+
+	return !!(val & PCI_EXP_LNKSTA_DLLLA);
+}
+
+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_establish_link(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-ipq8064", .data = &ops_v0 },
+	{ .compatible = "qcom,pcie-apq8064", .data = &ops_v0 },
+	{ .compatible = "qcom,pcie-apq8084", .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 <svarbanov@mm-sol.com>");
+MODULE_DESCRIPTION("Qualcomm PCIe root complex driver");
+MODULE_LICENSE("GPL v2");
-- 
1.7.9.5


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

* [PATCH v5 3/5] PCI: qcom: Add Qualcomm PCIe controller driver
@ 2015-12-18 12:38   ` Stanimir Varbanov
  0 siblings, 0 replies; 38+ messages in thread
From: Stanimir Varbanov @ 2015-12-18 12:38 UTC (permalink / raw)
  To: linux-arm-kernel

From: Stanimir Varbanov <svarbanov@mm-sol.com>

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>
Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 MAINTAINERS                  |    7 +
 drivers/pci/host/Kconfig     |   10 +
 drivers/pci/host/Makefile    |    1 +
 drivers/pci/host/pcie-qcom.c |  617 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 635 insertions(+)
 create mode 100644 drivers/pci/host/pcie-qcom.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 38df53f828e1..7de5d2597e2b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8247,6 +8247,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 <svarbanov@mm-sol.com>
+L:     linux-pci at vger.kernel.org
+L:     linux-arm-msm at vger.kernel.org
+S:     Maintained
+F:     drivers/pci/host/*qcom*
+
 PCMCIA SUBSYSTEM
 P:	Linux PCMCIA Team
 L:	linux-pcmcia at lists.infradead.org
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..36c81973467d
--- /dev/null
+++ b/drivers/pci/host/pcie-qcom.c
@@ -0,0 +1,617 @@
+/*
+ * 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_CAP				0x70
+
+#define PERST_DELAY_US				1000
+
+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);
+	usleep_range(PERST_DELAY_US, PERST_DELAY_US + 500);
+}
+
+static void qcom_ep_reset_deassert(struct qcom_pcie *pcie)
+{
+	gpiod_set_value(pcie->reset, 0);
+	usleep_range(PERST_DELAY_US, PERST_DELAY_US + 500);
+}
+
+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_establish_link(struct qcom_pcie *pcie)
+{
+	struct device *dev = pcie->dev;
+	unsigned int retries = 0;
+	u32 val;
+
+	if (dw_pcie_link_up(&pcie->pp))
+		return 0;
+
+	/* 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);
+
+	do {
+		if (dw_pcie_link_up(&pcie->pp))
+			return 0;
+		usleep_range(250, 1000);
+	} while (retries < 200);
+
+	dev_warn(dev, "phy link never came up\n");
+
+	return -ETIMEDOUT;
+}
+
+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);
+	u16 val = readw(pcie->dbi + PCIE20_CAP + PCI_EXP_LNKSTA);
+
+	return !!(val & PCI_EXP_LNKSTA_DLLLA);
+}
+
+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_establish_link(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-ipq8064", .data = &ops_v0 },
+	{ .compatible = "qcom,pcie-apq8064", .data = &ops_v0 },
+	{ .compatible = "qcom,pcie-apq8084", .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 <svarbanov@mm-sol.com>");
+MODULE_DESCRIPTION("Qualcomm PCIe root complex driver");
+MODULE_LICENSE("GPL v2");
-- 
1.7.9.5

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

* [PATCH v5 4/5] ARM: dts: apq8064: add pcie devicetree node
  2015-12-18 12:38 ` Stanimir Varbanov
@ 2015-12-18 12:38   ` Stanimir Varbanov
  -1 siblings, 0 replies; 38+ messages in thread
From: Stanimir Varbanov @ 2015-12-18 12:38 UTC (permalink / raw)
  To: linux-arm-msm, linux-kernel, linux-arm-kernel, devicetree,
	linux-pci, Bjorn Helgaas
  Cc: Srinivas Kandagatla, Russell King, Rob Herring, Rob Herring,
	Mark Rutland, Pawel Moll, Ian Campbell, Arnd Bergmann,
	Jingoo Han, Pratyush Anand, Bjorn Andersson, Stanimir Varbanov

Add the pcie dt node so that it can probe and used.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 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 1a57278cb818..495807eab8f3 100644
--- a/arch/arm/boot/dts/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
@@ -705,5 +705,41 @@
 			compatible = "qcom,tcsr-apq8064", "syscon";
 			reg = <0x1a400000 0x100>;
 		};
+
+		pcie: pci@1b500000 {
+			compatible = "qcom,pcie-apq8064", "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

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

* [PATCH v5 4/5] ARM: dts: apq8064: add pcie devicetree node
@ 2015-12-18 12:38   ` Stanimir Varbanov
  0 siblings, 0 replies; 38+ messages in thread
From: Stanimir Varbanov @ 2015-12-18 12:38 UTC (permalink / raw)
  To: linux-arm-kernel

Add the pcie dt node so that it can probe and used.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 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 1a57278cb818..495807eab8f3 100644
--- a/arch/arm/boot/dts/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
@@ -705,5 +705,41 @@
 			compatible = "qcom,tcsr-apq8064", "syscon";
 			reg = <0x1a400000 0x100>;
 		};
+
+		pcie: pci at 1b500000 {
+			compatible = "qcom,pcie-apq8064", "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

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

* [PATCH v5 5/5] ARM: dts: ifc6410: enable pcie dt node for this board
  2015-12-18 12:38 ` Stanimir Varbanov
@ 2015-12-18 12:38   ` Stanimir Varbanov
  -1 siblings, 0 replies; 38+ messages in thread
From: Stanimir Varbanov @ 2015-12-18 12:38 UTC (permalink / raw)
  To: linux-arm-msm, linux-kernel, linux-arm-kernel, devicetree,
	linux-pci, Bjorn Helgaas
  Cc: Srinivas Kandagatla, Russell King, Rob Herring, Rob Herring,
	Mark Rutland, Pawel Moll, Ian Campbell, Arnd Bergmann,
	Jingoo Han, Pratyush Anand, Bjorn Andersson, Stanimir Varbanov

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 <stanimir.varbanov@linaro.org>
---
 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 f40e93f09673..4584850add62 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;
 				};
 			};
+
+			pcie_pins: pcie_pinmux {
+				mux {
+					pins = "gpio27";
+					function = "gpio";
+				};
+				conf {
+					pins = "gpio27";
+					drive-strength = <12>;
+					bias-disable;
+				};
+			};
 		};
 
 		rpm@108000 {
@@ -123,6 +135,10 @@
 				lvs1 {
 					bias-pull-down;
 				};
+
+				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 = <&pcie_pins>;
+			pinctrl-names = "default";
+			perst-gpio = <&tlmm_pinmux 27 GPIO_ACTIVE_LOW>;
+		};
+
 		qcom,ssbi@500000 {
 			pmic@0 {
 				gpio@150 {
-- 
1.7.9.5

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

* [PATCH v5 5/5] ARM: dts: ifc6410: enable pcie dt node for this board
@ 2015-12-18 12:38   ` Stanimir Varbanov
  0 siblings, 0 replies; 38+ messages in thread
From: Stanimir Varbanov @ 2015-12-18 12:38 UTC (permalink / raw)
  To: linux-arm-kernel

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 <stanimir.varbanov@linaro.org>
---
 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 f40e93f09673..4584850add62 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;
 				};
 			};
+
+			pcie_pins: pcie_pinmux {
+				mux {
+					pins = "gpio27";
+					function = "gpio";
+				};
+				conf {
+					pins = "gpio27";
+					drive-strength = <12>;
+					bias-disable;
+				};
+			};
 		};
 
 		rpm at 108000 {
@@ -123,6 +135,10 @@
 				lvs1 {
 					bias-pull-down;
 				};
+
+				lvs6 {
+					bias-pull-down;
+				};
 			};
 		};
 
@@ -231,6 +247,16 @@
 			status = "okay";
 		};
 
+		pci at 1b500000 {
+			status = "ok";
+			vdda-supply = <&pm8921_s3>;
+			vdda_phy-supply = <&pm8921_lvs6>;
+			vdda_refclk-supply = <&ext_3p3v>;
+			pinctrl-0 = <&pcie_pins>;
+			pinctrl-names = "default";
+			perst-gpio = <&tlmm_pinmux 27 GPIO_ACTIVE_LOW>;
+		};
+
 		qcom,ssbi at 500000 {
 			pmic at 0 {
 				gpio at 150 {
-- 
1.7.9.5

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

* [PATCH] PCI: qcom: fix ptr_ret.cocci warnings
  2015-12-18 12:38   ` Stanimir Varbanov
  (?)
@ 2015-12-18 13:44     ` kbuild test robot
  -1 siblings, 0 replies; 38+ messages in thread
From: kbuild test robot @ 2015-12-18 13:44 UTC (permalink / raw)
  Cc: kbuild-all, linux-arm-msm, linux-kernel, linux-arm-kernel,
	devicetree, linux-pci, Bjorn Helgaas, Srinivas Kandagatla,
	Russell King, Rob Herring, Rob Herring, Mark Rutland, Pawel Moll,
	Ian Campbell, Arnd Bergmann, Jingoo Han, Pratyush Anand,
	Bjorn Andersson, Stanimir Varbanov, Stanimir Varbanov

drivers/pci/host/pcie-qcom.c:188:1-3: WARNING: PTR_ERR_OR_ZERO can be used
drivers/pci/host/pcie-qcom.c:220:1-3: WARNING: PTR_ERR_OR_ZERO can be used


 Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR

Generated by: scripts/coccinelle/api/ptr_ret.cocci

CC: Stanimir Varbanov <svarbanov@mm-sol.com>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

 pcie-qcom.c |   10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

--- a/drivers/pci/host/pcie-qcom.c
+++ b/drivers/pci/host/pcie-qcom.c
@@ -185,10 +185,7 @@ static int qcom_pcie_get_resources_v0(st
 		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;
+	return PTR_ERR_OR_ZERO(res->phy_reset);
 }
 
 static int qcom_pcie_get_resources_v1(struct qcom_pcie *pcie)
@@ -217,10 +214,7 @@ static int qcom_pcie_get_resources_v1(st
 		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;
+	return PTR_ERR_OR_ZERO(res->core);
 }
 
 static void qcom_pcie_deinit_v0(struct qcom_pcie *pcie)

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

* Re: [PATCH v5 3/5] PCI: qcom: Add Qualcomm PCIe controller driver
  2015-12-18 12:38   ` Stanimir Varbanov
  (?)
@ 2015-12-18 13:44     ` kbuild test robot
  -1 siblings, 0 replies; 38+ messages in thread
From: kbuild test robot @ 2015-12-18 13:44 UTC (permalink / raw)
  Cc: kbuild-all, linux-arm-msm, linux-kernel, linux-arm-kernel,
	devicetree, linux-pci, Bjorn Helgaas, Srinivas Kandagatla,
	Russell King, Rob Herring, Rob Herring, Mark Rutland, Pawel Moll,
	Ian Campbell, Arnd Bergmann, Jingoo Han, Pratyush Anand,
	Bjorn Andersson, Stanimir Varbanov, Stanimir Varbanov

Hi Stanimir,

[auto build test WARNING on next-20151218]
[also build test WARNING on v4.4-rc5]
[cannot apply to pci/next robh/for-next v4.4-rc5 v4.4-rc4 v4.4-rc3]

url:    https://github.com/0day-ci/linux/commits/Stanimir-Varbanov/Qualcomm-PCIe-driver-and-designware-fixes/20151218-205427


coccinelle warnings: (new ones prefixed by >>)

>> drivers/pci/host/pcie-qcom.c:188:1-3: WARNING: PTR_ERR_OR_ZERO can be used
   drivers/pci/host/pcie-qcom.c:220:1-3: WARNING: PTR_ERR_OR_ZERO can be used

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

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

* Re: [PATCH v5 3/5] PCI: qcom: Add Qualcomm PCIe controller driver
@ 2015-12-18 13:44     ` kbuild test robot
  0 siblings, 0 replies; 38+ messages in thread
From: kbuild test robot @ 2015-12-18 13:44 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: kbuild-all, linux-arm-msm, linux-kernel, linux-arm-kernel,
	devicetree, linux-pci, Bjorn Helgaas, Srinivas Kandagatla,
	Russell King, Rob Herring, Rob Herring, Mark Rutland, Pawel Moll,
	Ian Campbell, Arnd Bergmann, Jingoo Han, Pratyush Anand,
	Bjorn Andersson, Stanimir Varbanov, Stanimir Varbanov

Hi Stanimir,

[auto build test WARNING on next-20151218]
[also build test WARNING on v4.4-rc5]
[cannot apply to pci/next robh/for-next v4.4-rc5 v4.4-rc4 v4.4-rc3]

url:    https://github.com/0day-ci/linux/commits/Stanimir-Varbanov/Qualcomm-PCIe-driver-and-designware-fixes/20151218-205427


coccinelle warnings: (new ones prefixed by >>)

>> drivers/pci/host/pcie-qcom.c:188:1-3: WARNING: PTR_ERR_OR_ZERO can be used
   drivers/pci/host/pcie-qcom.c:220:1-3: WARNING: PTR_ERR_OR_ZERO can be used

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

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

* [PATCH] PCI: qcom: fix ptr_ret.cocci warnings
@ 2015-12-18 13:44     ` kbuild test robot
  0 siblings, 0 replies; 38+ messages in thread
From: kbuild test robot @ 2015-12-18 13:44 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: kbuild-all, linux-arm-msm, linux-kernel, linux-arm-kernel,
	devicetree, linux-pci, Bjorn Helgaas, Srinivas Kandagatla,
	Russell King, Rob Herring, Rob Herring, Mark Rutland, Pawel Moll,
	Ian Campbell, Arnd Bergmann, Jingoo Han, Pratyush Anand,
	Bjorn Andersson, Stanimir Varbanov, Stanimir Varbanov

drivers/pci/host/pcie-qcom.c:188:1-3: WARNING: PTR_ERR_OR_ZERO can be used
drivers/pci/host/pcie-qcom.c:220:1-3: WARNING: PTR_ERR_OR_ZERO can be used


 Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR

Generated by: scripts/coccinelle/api/ptr_ret.cocci

CC: Stanimir Varbanov <svarbanov@mm-sol.com>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

 pcie-qcom.c |   10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

--- a/drivers/pci/host/pcie-qcom.c
+++ b/drivers/pci/host/pcie-qcom.c
@@ -185,10 +185,7 @@ static int qcom_pcie_get_resources_v0(st
 		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;
+	return PTR_ERR_OR_ZERO(res->phy_reset);
 }
 
 static int qcom_pcie_get_resources_v1(struct qcom_pcie *pcie)
@@ -217,10 +214,7 @@ static int qcom_pcie_get_resources_v1(st
 		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;
+	return PTR_ERR_OR_ZERO(res->core);
 }
 
 static void qcom_pcie_deinit_v0(struct qcom_pcie *pcie)

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

* [PATCH v5 3/5] PCI: qcom: Add Qualcomm PCIe controller driver
@ 2015-12-18 13:44     ` kbuild test robot
  0 siblings, 0 replies; 38+ messages in thread
From: kbuild test robot @ 2015-12-18 13:44 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Stanimir,

[auto build test WARNING on next-20151218]
[also build test WARNING on v4.4-rc5]
[cannot apply to pci/next robh/for-next v4.4-rc5 v4.4-rc4 v4.4-rc3]

url:    https://github.com/0day-ci/linux/commits/Stanimir-Varbanov/Qualcomm-PCIe-driver-and-designware-fixes/20151218-205427


coccinelle warnings: (new ones prefixed by >>)

>> drivers/pci/host/pcie-qcom.c:188:1-3: WARNING: PTR_ERR_OR_ZERO can be used
   drivers/pci/host/pcie-qcom.c:220:1-3: WARNING: PTR_ERR_OR_ZERO can be used

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

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

* [PATCH] PCI: qcom: fix ptr_ret.cocci warnings
@ 2015-12-18 13:44     ` kbuild test robot
  0 siblings, 0 replies; 38+ messages in thread
From: kbuild test robot @ 2015-12-18 13:44 UTC (permalink / raw)
  To: linux-arm-kernel

drivers/pci/host/pcie-qcom.c:188:1-3: WARNING: PTR_ERR_OR_ZERO can be used
drivers/pci/host/pcie-qcom.c:220:1-3: WARNING: PTR_ERR_OR_ZERO can be used


 Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR

Generated by: scripts/coccinelle/api/ptr_ret.cocci

CC: Stanimir Varbanov <svarbanov@mm-sol.com>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

 pcie-qcom.c |   10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

--- a/drivers/pci/host/pcie-qcom.c
+++ b/drivers/pci/host/pcie-qcom.c
@@ -185,10 +185,7 @@ static int qcom_pcie_get_resources_v0(st
 		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;
+	return PTR_ERR_OR_ZERO(res->phy_reset);
 }
 
 static int qcom_pcie_get_resources_v1(struct qcom_pcie *pcie)
@@ -217,10 +214,7 @@ static int qcom_pcie_get_resources_v1(st
 		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;
+	return PTR_ERR_OR_ZERO(res->core);
 }
 
 static void qcom_pcie_deinit_v0(struct qcom_pcie *pcie)

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

* Re: [PATCH v5 1/5] PCI: designware: ensure ATU is enabled before IO/conf space accesses
  2015-12-18 12:38   ` Stanimir Varbanov
  (?)
@ 2015-12-18 14:41     ` Pratyush Anand
  -1 siblings, 0 replies; 38+ messages in thread
From: Pratyush Anand @ 2015-12-18 14:41 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: linux-arm-msm, linux-kernel, linux-arm-kernel, devicetree,
	linux-pci, Bjorn Helgaas, Srinivas Kandagatla, Russell King,
	Rob Herring, Rob Herring, Mark Rutland, Pawel Moll, Ian Campbell,
	Arnd Bergmann, Jingoo Han, Bjorn Andersson

On Fri, Dec 18, 2015 at 6:08 PM, Stanimir Varbanov
<stanimir.varbanov@linaro.org> wrote:
> There is no guarantees that enabling ATU will hit the hardware
> immediately, and subsequent accesses to configuration / IO spaces
> are reliable. So fixing this by read back PCIE_ATU_CR2 register
> just after writing.
>
> Without such a fix the 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 <stanimir.varbanov@linaro.org>

Acked-by: Pratyush Anand <pratyush.anand@gmail.com>

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

* Re: [PATCH v5 1/5] PCI: designware: ensure ATU is enabled before IO/conf space accesses
@ 2015-12-18 14:41     ` Pratyush Anand
  0 siblings, 0 replies; 38+ messages in thread
From: Pratyush Anand @ 2015-12-18 14:41 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: linux-arm-msm, linux-kernel, linux-arm-kernel, devicetree,
	linux-pci, Bjorn Helgaas, Srinivas Kandagatla, Russell King,
	Rob Herring, Rob Herring, Mark Rutland, Pawel Moll, Ian Campbell,
	Arnd Bergmann, Jingoo Han, Bjorn Andersson

On Fri, Dec 18, 2015 at 6:08 PM, Stanimir Varbanov
<stanimir.varbanov@linaro.org> wrote:
> There is no guarantees that enabling ATU will hit the hardware
> immediately, and subsequent accesses to configuration / IO spaces
> are reliable. So fixing this by read back PCIE_ATU_CR2 register
> just after writing.
>
> Without such a fix the 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 <stanimir.varbanov@linaro.org>

Acked-by: Pratyush Anand <pratyush.anand@gmail.com>

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

* [PATCH v5 1/5] PCI: designware: ensure ATU is enabled before IO/conf space accesses
@ 2015-12-18 14:41     ` Pratyush Anand
  0 siblings, 0 replies; 38+ messages in thread
From: Pratyush Anand @ 2015-12-18 14:41 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Dec 18, 2015 at 6:08 PM, Stanimir Varbanov
<stanimir.varbanov@linaro.org> wrote:
> There is no guarantees that enabling ATU will hit the hardware
> immediately, and subsequent accesses to configuration / IO spaces
> are reliable. So fixing this by read back PCIE_ATU_CR2 register
> just after writing.
>
> Without such a fix the 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 <stanimir.varbanov@linaro.org>

Acked-by: Pratyush Anand <pratyush.anand@gmail.com>

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

* Re: [PATCH v5 3/5] PCI: qcom: Add Qualcomm PCIe controller driver
  2015-12-18 13:44     ` kbuild test robot
  (?)
@ 2015-12-20 23:10       ` Bjorn Andersson
  -1 siblings, 0 replies; 38+ messages in thread
From: Bjorn Andersson @ 2015-12-20 23:10 UTC (permalink / raw)
  To: kbuild test robot
  Cc: Stanimir Varbanov, kbuild-all, linux-arm-msm, linux-kernel,
	linux-arm-kernel, devicetree, linux-pci, Bjorn Helgaas,
	Srinivas Kandagatla, Russell King, Rob Herring, Rob Herring,
	Mark Rutland, Pawel Moll, Ian Campbell, Arnd Bergmann,
	Jingoo Han, Pratyush Anand, Bjorn Andersson, Stanimir Varbanov

On Fri, Dec 18, 2015 at 5:44 AM, kbuild test robot <lkp@intel.com> wrote:
> Hi Stanimir,
>
> [auto build test WARNING on next-20151218]
> [also build test WARNING on v4.4-rc5]
> [cannot apply to pci/next robh/for-next v4.4-rc5 v4.4-rc4 v4.4-rc3]
>
> url:    https://github.com/0day-ci/linux/commits/Stanimir-Varbanov/Qualcomm-PCIe-driver-and-designware-fixes/20151218-205427
>
>
> coccinelle warnings: (new ones prefixed by >>)
>
>>> drivers/pci/host/pcie-qcom.c:188:1-3: WARNING: PTR_ERR_OR_ZERO can be used
>    drivers/pci/host/pcie-qcom.c:220:1-3: WARNING: PTR_ERR_OR_ZERO can be used
>
> Please review and possibly fold the followup patch.
>

I disagree with this "recommendation" as it's only outcome will be asymmetry.

I think this script should be changed to only warn if there's a single
IS_ERR/PTR_ERR at the end of the function, not if there's a list of
them and this would replace the last one.

Regards,
Bjorn

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

* Re: [PATCH v5 3/5] PCI: qcom: Add Qualcomm PCIe controller driver
@ 2015-12-20 23:10       ` Bjorn Andersson
  0 siblings, 0 replies; 38+ messages in thread
From: Bjorn Andersson @ 2015-12-20 23:10 UTC (permalink / raw)
  To: kbuild test robot
  Cc: Stanimir Varbanov, kbuild-all, linux-arm-msm, linux-kernel,
	linux-arm-kernel, devicetree, linux-pci, Bjorn Helgaas,
	Srinivas Kandagatla, Russell King, Rob Herring, Rob Herring,
	Mark Rutland, Pawel Moll, Ian Campbell, Arnd Bergmann,
	Jingoo Han, Pratyush Anand, Bjorn Andersson, Stanimir Varbanov

On Fri, Dec 18, 2015 at 5:44 AM, kbuild test robot <lkp@intel.com> wrote:
> Hi Stanimir,
>
> [auto build test WARNING on next-20151218]
> [also build test WARNING on v4.4-rc5]
> [cannot apply to pci/next robh/for-next v4.4-rc5 v4.4-rc4 v4.4-rc3]
>
> url:    https://github.com/0day-ci/linux/commits/Stanimir-Varbanov/Qualcomm-PCIe-driver-and-designware-fixes/20151218-205427
>
>
> coccinelle warnings: (new ones prefixed by >>)
>
>>> drivers/pci/host/pcie-qcom.c:188:1-3: WARNING: PTR_ERR_OR_ZERO can be used
>    drivers/pci/host/pcie-qcom.c:220:1-3: WARNING: PTR_ERR_OR_ZERO can be used
>
> Please review and possibly fold the followup patch.
>

I disagree with this "recommendation" as it's only outcome will be asymmetry.

I think this script should be changed to only warn if there's a single
IS_ERR/PTR_ERR at the end of the function, not if there's a list of
them and this would replace the last one.

Regards,
Bjorn

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

* [PATCH v5 3/5] PCI: qcom: Add Qualcomm PCIe controller driver
@ 2015-12-20 23:10       ` Bjorn Andersson
  0 siblings, 0 replies; 38+ messages in thread
From: Bjorn Andersson @ 2015-12-20 23:10 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Dec 18, 2015 at 5:44 AM, kbuild test robot <lkp@intel.com> wrote:
> Hi Stanimir,
>
> [auto build test WARNING on next-20151218]
> [also build test WARNING on v4.4-rc5]
> [cannot apply to pci/next robh/for-next v4.4-rc5 v4.4-rc4 v4.4-rc3]
>
> url:    https://github.com/0day-ci/linux/commits/Stanimir-Varbanov/Qualcomm-PCIe-driver-and-designware-fixes/20151218-205427
>
>
> coccinelle warnings: (new ones prefixed by >>)
>
>>> drivers/pci/host/pcie-qcom.c:188:1-3: WARNING: PTR_ERR_OR_ZERO can be used
>    drivers/pci/host/pcie-qcom.c:220:1-3: WARNING: PTR_ERR_OR_ZERO can be used
>
> Please review and possibly fold the followup patch.
>

I disagree with this "recommendation" as it's only outcome will be asymmetry.

I think this script should be changed to only warn if there's a single
IS_ERR/PTR_ERR at the end of the function, not if there's a list of
them and this would replace the last one.

Regards,
Bjorn

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

* Re: [PATCH v5 3/5] PCI: qcom: Add Qualcomm PCIe controller driver
  2015-12-20 23:10       ` Bjorn Andersson
  (?)
@ 2015-12-21 23:04         ` Arnd Bergmann
  -1 siblings, 0 replies; 38+ messages in thread
From: Arnd Bergmann @ 2015-12-21 23:04 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: kbuild test robot, Stanimir Varbanov, kbuild-all, linux-arm-msm,
	linux-kernel, linux-arm-kernel, devicetree, linux-pci,
	Bjorn Helgaas, Srinivas Kandagatla, Russell King, Rob Herring,
	Rob Herring, Mark Rutland, Pawel Moll, Ian Campbell, Jingoo Han,
	Pratyush Anand, Bjorn Andersson, Stanimir Varbanov

On Monday 21 December 2015, Bjorn Andersson wrote:
> I disagree with this "recommendation" as it's only outcome will be asymmetry.
> 
> I think this script should be changed to only warn if there's a single
> IS_ERR/PTR_ERR at the end of the function, not if there's a list of
> them and this would replace the last one.
> 

Agreed, looks like a false positive, and I think it's best to ignore the
warning here, but not change the script that seems reasonable in general.

	Arnd

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

* Re: [PATCH v5 3/5] PCI: qcom: Add Qualcomm PCIe controller driver
@ 2015-12-21 23:04         ` Arnd Bergmann
  0 siblings, 0 replies; 38+ messages in thread
From: Arnd Bergmann @ 2015-12-21 23:04 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: kbuild test robot, Stanimir Varbanov, kbuild-all, linux-arm-msm,
	linux-kernel, linux-arm-kernel, devicetree, linux-pci,
	Bjorn Helgaas, Srinivas Kandagatla, Russell King, Rob Herring,
	Rob Herring, Mark Rutland, Pawel Moll, Ian Campbell, Jingoo Han,
	Pratyush Anand, Bjorn Andersson, Stanimir Varbanov

On Monday 21 December 2015, Bjorn Andersson wrote:
> I disagree with this "recommendation" as it's only outcome will be asymmetry.
> 
> I think this script should be changed to only warn if there's a single
> IS_ERR/PTR_ERR at the end of the function, not if there's a list of
> them and this would replace the last one.
> 

Agreed, looks like a false positive, and I think it's best to ignore the
warning here, but not change the script that seems reasonable in general.

	Arnd

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

* [PATCH v5 3/5] PCI: qcom: Add Qualcomm PCIe controller driver
@ 2015-12-21 23:04         ` Arnd Bergmann
  0 siblings, 0 replies; 38+ messages in thread
From: Arnd Bergmann @ 2015-12-21 23:04 UTC (permalink / raw)
  To: linux-arm-kernel

On Monday 21 December 2015, Bjorn Andersson wrote:
> I disagree with this "recommendation" as it's only outcome will be asymmetry.
> 
> I think this script should be changed to only warn if there's a single
> IS_ERR/PTR_ERR at the end of the function, not if there's a list of
> them and this would replace the last one.
> 

Agreed, looks like a false positive, and I think it's best to ignore the
warning here, but not change the script that seems reasonable in general.

	Arnd

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

* Re: [PATCH v5 1/5] PCI: designware: ensure ATU is enabled before IO/conf space accesses
  2015-12-18 14:41     ` Pratyush Anand
  (?)
@ 2016-01-04 14:31       ` Stanimir Varbanov
  -1 siblings, 0 replies; 38+ messages in thread
From: Stanimir Varbanov @ 2016-01-04 14:31 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Pratyush Anand, Stanimir Varbanov, linux-arm-msm, linux-kernel,
	linux-arm-kernel, devicetree, linux-pci, Srinivas Kandagatla,
	Russell King, Rob Herring, Rob Herring, Mark Rutland, Pawel Moll,
	Ian Campbell, Arnd Bergmann, Jingoo Han, Bjorn Andersson

On 12/18/2015 04:41 PM, Pratyush Anand wrote:
> On Fri, Dec 18, 2015 at 6:08 PM, Stanimir Varbanov
> <stanimir.varbanov@linaro.org> wrote:
>> There is no guarantees that enabling ATU will hit the hardware
>> immediately, and subsequent accesses to configuration / IO spaces
>> are reliable. So fixing this by read back PCIE_ATU_CR2 register
>> just after writing.
>>
>> Without such a fix the 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 <stanimir.varbanov@linaro.org>
> 
> Acked-by: Pratyush Anand <pratyush.anand@gmail.com>

Bjorn, Do you want to resend the whole patchset with collected acks and
tested tags?


-- 
regards,
Stan

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

* Re: [PATCH v5 1/5] PCI: designware: ensure ATU is enabled before IO/conf space accesses
@ 2016-01-04 14:31       ` Stanimir Varbanov
  0 siblings, 0 replies; 38+ messages in thread
From: Stanimir Varbanov @ 2016-01-04 14:31 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Pratyush Anand, Stanimir Varbanov, linux-arm-msm, linux-kernel,
	linux-arm-kernel, devicetree, linux-pci, Srinivas Kandagatla,
	Russell King, Rob Herring, Rob Herring, Mark Rutland, Pawel Moll,
	Ian Campbell, Arnd Bergmann, Jingoo Han, Bjorn Andersson

On 12/18/2015 04:41 PM, Pratyush Anand wrote:
> On Fri, Dec 18, 2015 at 6:08 PM, Stanimir Varbanov
> <stanimir.varbanov@linaro.org> wrote:
>> There is no guarantees that enabling ATU will hit the hardware
>> immediately, and subsequent accesses to configuration / IO spaces
>> are reliable. So fixing this by read back PCIE_ATU_CR2 register
>> just after writing.
>>
>> Without such a fix the 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 <stanimir.varbanov@linaro.org>
> 
> Acked-by: Pratyush Anand <pratyush.anand@gmail.com>

Bjorn, Do you want to resend the whole patchset with collected acks and
tested tags?


-- 
regards,
Stan

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

* [PATCH v5 1/5] PCI: designware: ensure ATU is enabled before IO/conf space accesses
@ 2016-01-04 14:31       ` Stanimir Varbanov
  0 siblings, 0 replies; 38+ messages in thread
From: Stanimir Varbanov @ 2016-01-04 14:31 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/18/2015 04:41 PM, Pratyush Anand wrote:
> On Fri, Dec 18, 2015 at 6:08 PM, Stanimir Varbanov
> <stanimir.varbanov@linaro.org> wrote:
>> There is no guarantees that enabling ATU will hit the hardware
>> immediately, and subsequent accesses to configuration / IO spaces
>> are reliable. So fixing this by read back PCIE_ATU_CR2 register
>> just after writing.
>>
>> Without such a fix the 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 <stanimir.varbanov@linaro.org>
> 
> Acked-by: Pratyush Anand <pratyush.anand@gmail.com>

Bjorn, Do you want to resend the whole patchset with collected acks and
tested tags?


-- 
regards,
Stan

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

* Re: [PATCH v5 0/5] Qualcomm PCIe driver and designware fixes
  2015-12-18 12:38 ` Stanimir Varbanov
@ 2016-01-05 21:42   ` Bjorn Helgaas
  -1 siblings, 0 replies; 38+ messages in thread
From: Bjorn Helgaas @ 2016-01-05 21:42 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: linux-arm-msm, linux-kernel, linux-arm-kernel, devicetree,
	linux-pci, Bjorn Helgaas, Srinivas Kandagatla, Russell King,
	Rob Herring, Rob Herring, Mark Rutland, Pawel Moll, Ian Campbell,
	Arnd Bergmann, Jingoo Han, Pratyush Anand, Bjorn Andersson

On Fri, Dec 18, 2015 at 02:38:54PM +0200, Stanimir Varbanov wrote:
> Hi,
> 
> I'm sending v5 with following changes:
> - in 1/5 - replace wmb() call with PCIE_ATU_CR2 register read.
> - in 3/5 - addressed comments from Born Helgaas about usage of a
> standard PCI capabilities register names and rename link training
> function to the way that other Designware based drivers use.
> - in 5/5 - addressed a comment from Bjorn Andersson about regulator
> label duplication.
> 
> Comments are welcome!
> 
> The previous v4 of the patch set can be found at [1].
> 
> regards,
> Stan
> 
> [1] https://lkml.org/lkml/2015/12/3/370
> 
> Stanimir Varbanov (5):
>   PCI: designware: ensure ATU is enabled before IO/conf space accesses
>   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          |  233 ++++++++
>  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                 |    7 +
>  drivers/pci/host/pcie-qcom.c                       |  617 ++++++++++++++++++++
>  8 files changed, 937 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/qcom,pcie.txt
>  create mode 100644 drivers/pci/host/pcie-qcom.c

Applied to pci/host-qcom for v4.5, with Pratyush's ack on the first patch.
Thanks!

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

* [PATCH v5 0/5] Qualcomm PCIe driver and designware fixes
@ 2016-01-05 21:42   ` Bjorn Helgaas
  0 siblings, 0 replies; 38+ messages in thread
From: Bjorn Helgaas @ 2016-01-05 21:42 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Dec 18, 2015 at 02:38:54PM +0200, Stanimir Varbanov wrote:
> Hi,
> 
> I'm sending v5 with following changes:
> - in 1/5 - replace wmb() call with PCIE_ATU_CR2 register read.
> - in 3/5 - addressed comments from Born Helgaas about usage of a
> standard PCI capabilities register names and rename link training
> function to the way that other Designware based drivers use.
> - in 5/5 - addressed a comment from Bjorn Andersson about regulator
> label duplication.
> 
> Comments are welcome!
> 
> The previous v4 of the patch set can be found at [1].
> 
> regards,
> Stan
> 
> [1] https://lkml.org/lkml/2015/12/3/370
> 
> Stanimir Varbanov (5):
>   PCI: designware: ensure ATU is enabled before IO/conf space accesses
>   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          |  233 ++++++++
>  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                 |    7 +
>  drivers/pci/host/pcie-qcom.c                       |  617 ++++++++++++++++++++
>  8 files changed, 937 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/qcom,pcie.txt
>  create mode 100644 drivers/pci/host/pcie-qcom.c

Applied to pci/host-qcom for v4.5, with Pratyush's ack on the first patch.
Thanks!

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

* Re: [PATCH v5 1/5] PCI: designware: ensure ATU is enabled before IO/conf space accesses
  2015-12-18 12:38   ` Stanimir Varbanov
@ 2016-01-06 18:20     ` Bjorn Helgaas
  -1 siblings, 0 replies; 38+ messages in thread
From: Bjorn Helgaas @ 2016-01-06 18:20 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: linux-arm-msm, linux-kernel, linux-arm-kernel, devicetree,
	linux-pci, Bjorn Helgaas, Srinivas Kandagatla, Russell King,
	Rob Herring, Rob Herring, Mark Rutland, Pawel Moll, Ian Campbell,
	Arnd Bergmann, Jingoo Han, Pratyush Anand, Bjorn Andersson,
	Jisheng Zhang

[+cc Jisheng]

On Fri, Dec 18, 2015 at 02:38:55PM +0200, Stanimir Varbanov wrote:
> There is no guarantees that enabling ATU will hit the hardware
> immediately, and subsequent accesses to configuration / IO spaces
> are reliable. So fixing this by read back PCIE_ATU_CR2 register
> just after writing.
> 
> Without such a fix the 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 <stanimir.varbanov@linaro.org>
> ---
>  drivers/pci/host/pcie-designware.c |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index 02a7452bdf23..7880de63895d 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -154,6 +154,8 @@ static int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
>  static void dw_pcie_prog_outbound_atu(struct pcie_port *pp, int index,
>  		int type, u64 cpu_addr, u64 pci_addr, u32 size)
>  {
> +	u32 val;
> +
>  	dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | index,
>  			  PCIE_ATU_VIEWPORT);
>  	dw_pcie_writel_rc(pp, lower_32_bits(cpu_addr), PCIE_ATU_LOWER_BASE);
> @@ -164,6 +166,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].
> +	 */
> +	dw_pcie_readl_rc(pp, PCIE_ATU_CR2, &val);

This particular fix makes sense to me.

But I have a larger question about how the ATU works.  I see these
definitions:

  #define PCIE_ATU_TYPE_MEM
  #define PCIE_ATU_TYPE_IO
  #define PCIE_ATU_TYPE_CFG0
  #define PCIE_ATU_TYPE_CFG1

and these uses:

  - In dw_pcie_host_init(), set PCIE_ATU_TYPE_MEM for unit 1
    (but only if rd_other_conf is not overridden)

  - In dw_pcie_rd_other_conf() and dw_pcie_wr_other_conf(),
    set PCIE_ATU_TYPE_CFG0 before config access to own bus;
    set PCIE_ATU_TYPE_CFG1 before config access to other bus;
    set PCIE_ATU_TYPE_IO after completion

I'm confused:

1) I assume PCIE_ATU_TYPE_MEM is for access to PCI memory space.  Why
is that initialization related to rd_other_conf?  Shouldn't that be
set up always?  A comment here would be nice, to clarify that this is
not related to the subsequent dw_pcie_wr_own_conf() calls.

2) Why doesn't dw_pcie_host_init() use dw_pcie_rd_conf() instead of
dw_pcie_rd_own_conf()?  Using pci_read_config_dword() might be even
better.  Using the internal interfaces piecemeal like we do today is
just an opportunity for doing it wrong.

3) The definitions and your comment about "accessing PCI
configuration/io spaces" suggest that the ATU must be programmed
differently for accesses to PCI config space vs. I/O space.  If that's
the case, we'd need some kind of mutex to protect inl(), etc., during
config accesses.  For example, in this scenario:

  thread A                                         thread B
  ---------------------------------------------    ----------
  pci_read_config_dword()
  dw_pcie_prog_outbound_atu(PCIE_ATU_TYPE_CFG0)
                                                   inl()
  dw_pcie_cfg_read()
  readl()
  dw_pcie_prog_outbound_atu(PCIE_ATU_TYPE_IO)
                                                   inl()

Do both inl() calls by thread B work correctly, even though the ATU
seems to be programmed for CFG0 for the first but IO for the second?

Bjorn

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

* [PATCH v5 1/5] PCI: designware: ensure ATU is enabled before IO/conf space accesses
@ 2016-01-06 18:20     ` Bjorn Helgaas
  0 siblings, 0 replies; 38+ messages in thread
From: Bjorn Helgaas @ 2016-01-06 18:20 UTC (permalink / raw)
  To: linux-arm-kernel

[+cc Jisheng]

On Fri, Dec 18, 2015 at 02:38:55PM +0200, Stanimir Varbanov wrote:
> There is no guarantees that enabling ATU will hit the hardware
> immediately, and subsequent accesses to configuration / IO spaces
> are reliable. So fixing this by read back PCIE_ATU_CR2 register
> just after writing.
> 
> Without such a fix the 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 <stanimir.varbanov@linaro.org>
> ---
>  drivers/pci/host/pcie-designware.c |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index 02a7452bdf23..7880de63895d 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -154,6 +154,8 @@ static int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
>  static void dw_pcie_prog_outbound_atu(struct pcie_port *pp, int index,
>  		int type, u64 cpu_addr, u64 pci_addr, u32 size)
>  {
> +	u32 val;
> +
>  	dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | index,
>  			  PCIE_ATU_VIEWPORT);
>  	dw_pcie_writel_rc(pp, lower_32_bits(cpu_addr), PCIE_ATU_LOWER_BASE);
> @@ -164,6 +166,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].
> +	 */
> +	dw_pcie_readl_rc(pp, PCIE_ATU_CR2, &val);

This particular fix makes sense to me.

But I have a larger question about how the ATU works.  I see these
definitions:

  #define PCIE_ATU_TYPE_MEM
  #define PCIE_ATU_TYPE_IO
  #define PCIE_ATU_TYPE_CFG0
  #define PCIE_ATU_TYPE_CFG1

and these uses:

  - In dw_pcie_host_init(), set PCIE_ATU_TYPE_MEM for unit 1
    (but only if rd_other_conf is not overridden)

  - In dw_pcie_rd_other_conf() and dw_pcie_wr_other_conf(),
    set PCIE_ATU_TYPE_CFG0 before config access to own bus;
    set PCIE_ATU_TYPE_CFG1 before config access to other bus;
    set PCIE_ATU_TYPE_IO after completion

I'm confused:

1) I assume PCIE_ATU_TYPE_MEM is for access to PCI memory space.  Why
is that initialization related to rd_other_conf?  Shouldn't that be
set up always?  A comment here would be nice, to clarify that this is
not related to the subsequent dw_pcie_wr_own_conf() calls.

2) Why doesn't dw_pcie_host_init() use dw_pcie_rd_conf() instead of
dw_pcie_rd_own_conf()?  Using pci_read_config_dword() might be even
better.  Using the internal interfaces piecemeal like we do today is
just an opportunity for doing it wrong.

3) The definitions and your comment about "accessing PCI
configuration/io spaces" suggest that the ATU must be programmed
differently for accesses to PCI config space vs. I/O space.  If that's
the case, we'd need some kind of mutex to protect inl(), etc., during
config accesses.  For example, in this scenario:

  thread A                                         thread B
  ---------------------------------------------    ----------
  pci_read_config_dword()
  dw_pcie_prog_outbound_atu(PCIE_ATU_TYPE_CFG0)
                                                   inl()
  dw_pcie_cfg_read()
  readl()
  dw_pcie_prog_outbound_atu(PCIE_ATU_TYPE_IO)
                                                   inl()

Do both inl() calls by thread B work correctly, even though the ATU
seems to be programmed for CFG0 for the first but IO for the second?

Bjorn

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

* Re: [PATCH v5 1/5] PCI: designware: ensure ATU is enabled before IO/conf space accesses
  2016-01-06 18:20     ` Bjorn Helgaas
  (?)
@ 2016-01-07  6:33       ` Jisheng Zhang
  -1 siblings, 0 replies; 38+ messages in thread
From: Jisheng Zhang @ 2016-01-07  6:33 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Stanimir Varbanov, linux-arm-msm, linux-kernel, linux-arm-kernel,
	devicetree, linux-pci, Bjorn Helgaas, Srinivas Kandagatla,
	Russell King, Rob Herring, Rob Herring, Mark Rutland, Pawel Moll,
	Ian Campbell, Arnd Bergmann, Jingoo Han, Pratyush Anand,
	Bjorn Andersson

Dear Bjorn,

On Wed, 6 Jan 2016 12:20:03 -0600 Bjorn Helgaas wrote:

> [+cc Jisheng]
> 
> On Fri, Dec 18, 2015 at 02:38:55PM +0200, Stanimir Varbanov wrote:
> > There is no guarantees that enabling ATU will hit the hardware
> > immediately, and subsequent accesses to configuration / IO spaces
> > are reliable. So fixing this by read back PCIE_ATU_CR2 register
> > just after writing.
> > 
> > Without such a fix the 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 <stanimir.varbanov@linaro.org>
> > ---
> >  drivers/pci/host/pcie-designware.c |    7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> > index 02a7452bdf23..7880de63895d 100644
> > --- a/drivers/pci/host/pcie-designware.c
> > +++ b/drivers/pci/host/pcie-designware.c
> > @@ -154,6 +154,8 @@ static int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
> >  static void dw_pcie_prog_outbound_atu(struct pcie_port *pp, int index,
> >  		int type, u64 cpu_addr, u64 pci_addr, u32 size)
> >  {
> > +	u32 val;
> > +
> >  	dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | index,
> >  			  PCIE_ATU_VIEWPORT);
> >  	dw_pcie_writel_rc(pp, lower_32_bits(cpu_addr), PCIE_ATU_LOWER_BASE);
> > @@ -164,6 +166,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].
> > +	 */
> > +	dw_pcie_readl_rc(pp, PCIE_ATU_CR2, &val);  
> 
> This particular fix makes sense to me.
> 
> But I have a larger question about how the ATU works.  I see these
> definitions:
> 
>   #define PCIE_ATU_TYPE_MEM
>   #define PCIE_ATU_TYPE_IO
>   #define PCIE_ATU_TYPE_CFG0
>   #define PCIE_ATU_TYPE_CFG1
> 
> and these uses:
> 
>   - In dw_pcie_host_init(), set PCIE_ATU_TYPE_MEM for unit 1
>     (but only if rd_other_conf is not overridden)
> 
>   - In dw_pcie_rd_other_conf() and dw_pcie_wr_other_conf(),
>     set PCIE_ATU_TYPE_CFG0 before config access to own bus;
>     set PCIE_ATU_TYPE_CFG1 before config access to other bus;
>     set PCIE_ATU_TYPE_IO after completion
> 
> I'm confused:
> 
> 1) I assume PCIE_ATU_TYPE_MEM is for access to PCI memory space.  Why
> is that initialization related to rd_other_conf?  Shouldn't that be
> set up always?  A comment here would be nice, to clarify that this is
> not related to the subsequent dw_pcie_wr_own_conf() calls.

Indeed, the comment is necessary. I forget the reason until read the code
carefully again. The reason here is

If the platform provides ->rd_other_conf, it means the platform
doesn't support ATU, it uses its own address translation component
rather than ATU, so we should ignore ATU programming for this
kind of platform.

I have sent out one patch to add this comment.

> 
> 2) Why doesn't dw_pcie_host_init() use dw_pcie_rd_conf() instead of
> dw_pcie_rd_own_conf()?  Using pci_read_config_dword() might be even
> better.  Using the internal interfaces piecemeal like we do today is
> just an opportunity for doing it wrong.

IMHO, the reason is during host_init, the pci bus etc. are not initialized,
from another side, the code is really accessing its own conf registers.

> 
> 3) The definitions and your comment about "accessing PCI
> configuration/io spaces" suggest that the ATU must be programmed
> differently for accesses to PCI config space vs. I/O space.  If that's

Yes.

> the case, we'd need some kind of mutex to protect inl(), etc., during
> config accesses.  For example, in this scenario:
> 
>   thread A                                         thread B
>   ---------------------------------------------    ----------
>   pci_read_config_dword()
>   dw_pcie_prog_outbound_atu(PCIE_ATU_TYPE_CFG0)
>                                                    inl()
>   dw_pcie_cfg_read()
>   readl()
>   dw_pcie_prog_outbound_atu(PCIE_ATU_TYPE_IO)
>                                                    inl()
> 
> Do both inl() calls by thread B work correctly, even though the ATU
> seems to be programmed for CFG0 for the first but IO for the second?
> 

Indeed, there's such race issue as you and RMK pointed out.

IMHO, we need support:

1. platforms which contain three or more iATUs, there's no need to mux
iATU usage: one ATU for IO, one for cfg and another for MEM. But how to
get the number of iATUs, DT? eg. "snps,atu_num = 3"

2. platforms which contain only two iATUs, add mechanism to protect
the cfg vs IO race. Could you please give suggestions to how to achieve
this goal?

Thanks,
Jisheng

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

* Re: [PATCH v5 1/5] PCI: designware: ensure ATU is enabled before IO/conf space accesses
@ 2016-01-07  6:33       ` Jisheng Zhang
  0 siblings, 0 replies; 38+ messages in thread
From: Jisheng Zhang @ 2016-01-07  6:33 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Stanimir Varbanov, linux-arm-msm, linux-kernel, linux-arm-kernel,
	devicetree, linux-pci, Bjorn Helgaas, Srinivas Kandagatla,
	Russell King, Rob Herring, Rob Herring, Mark Rutland, Pawel Moll,
	Ian Campbell, Arnd Bergmann, Jingoo Han, Pratyush Anand,
	Bjorn Andersson

Dear Bjorn,

On Wed, 6 Jan 2016 12:20:03 -0600 Bjorn Helgaas wrote:

> [+cc Jisheng]
> 
> On Fri, Dec 18, 2015 at 02:38:55PM +0200, Stanimir Varbanov wrote:
> > There is no guarantees that enabling ATU will hit the hardware
> > immediately, and subsequent accesses to configuration / IO spaces
> > are reliable. So fixing this by read back PCIE_ATU_CR2 register
> > just after writing.
> > 
> > Without such a fix the 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 <stanimir.varbanov@linaro.org>
> > ---
> >  drivers/pci/host/pcie-designware.c |    7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> > index 02a7452bdf23..7880de63895d 100644
> > --- a/drivers/pci/host/pcie-designware.c
> > +++ b/drivers/pci/host/pcie-designware.c
> > @@ -154,6 +154,8 @@ static int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
> >  static void dw_pcie_prog_outbound_atu(struct pcie_port *pp, int index,
> >  		int type, u64 cpu_addr, u64 pci_addr, u32 size)
> >  {
> > +	u32 val;
> > +
> >  	dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | index,
> >  			  PCIE_ATU_VIEWPORT);
> >  	dw_pcie_writel_rc(pp, lower_32_bits(cpu_addr), PCIE_ATU_LOWER_BASE);
> > @@ -164,6 +166,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].
> > +	 */
> > +	dw_pcie_readl_rc(pp, PCIE_ATU_CR2, &val);  
> 
> This particular fix makes sense to me.
> 
> But I have a larger question about how the ATU works.  I see these
> definitions:
> 
>   #define PCIE_ATU_TYPE_MEM
>   #define PCIE_ATU_TYPE_IO
>   #define PCIE_ATU_TYPE_CFG0
>   #define PCIE_ATU_TYPE_CFG1
> 
> and these uses:
> 
>   - In dw_pcie_host_init(), set PCIE_ATU_TYPE_MEM for unit 1
>     (but only if rd_other_conf is not overridden)
> 
>   - In dw_pcie_rd_other_conf() and dw_pcie_wr_other_conf(),
>     set PCIE_ATU_TYPE_CFG0 before config access to own bus;
>     set PCIE_ATU_TYPE_CFG1 before config access to other bus;
>     set PCIE_ATU_TYPE_IO after completion
> 
> I'm confused:
> 
> 1) I assume PCIE_ATU_TYPE_MEM is for access to PCI memory space.  Why
> is that initialization related to rd_other_conf?  Shouldn't that be
> set up always?  A comment here would be nice, to clarify that this is
> not related to the subsequent dw_pcie_wr_own_conf() calls.

Indeed, the comment is necessary. I forget the reason until read the code
carefully again. The reason here is

If the platform provides ->rd_other_conf, it means the platform
doesn't support ATU, it uses its own address translation component
rather than ATU, so we should ignore ATU programming for this
kind of platform.

I have sent out one patch to add this comment.

> 
> 2) Why doesn't dw_pcie_host_init() use dw_pcie_rd_conf() instead of
> dw_pcie_rd_own_conf()?  Using pci_read_config_dword() might be even
> better.  Using the internal interfaces piecemeal like we do today is
> just an opportunity for doing it wrong.

IMHO, the reason is during host_init, the pci bus etc. are not initialized,
from another side, the code is really accessing its own conf registers.

> 
> 3) The definitions and your comment about "accessing PCI
> configuration/io spaces" suggest that the ATU must be programmed
> differently for accesses to PCI config space vs. I/O space.  If that's

Yes.

> the case, we'd need some kind of mutex to protect inl(), etc., during
> config accesses.  For example, in this scenario:
> 
>   thread A                                         thread B
>   ---------------------------------------------    ----------
>   pci_read_config_dword()
>   dw_pcie_prog_outbound_atu(PCIE_ATU_TYPE_CFG0)
>                                                    inl()
>   dw_pcie_cfg_read()
>   readl()
>   dw_pcie_prog_outbound_atu(PCIE_ATU_TYPE_IO)
>                                                    inl()
> 
> Do both inl() calls by thread B work correctly, even though the ATU
> seems to be programmed for CFG0 for the first but IO for the second?
> 

Indeed, there's such race issue as you and RMK pointed out.

IMHO, we need support:

1. platforms which contain three or more iATUs, there's no need to mux
iATU usage: one ATU for IO, one for cfg and another for MEM. But how to
get the number of iATUs, DT? eg. "snps,atu_num = 3"

2. platforms which contain only two iATUs, add mechanism to protect
the cfg vs IO race. Could you please give suggestions to how to achieve
this goal?

Thanks,
Jisheng


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

* [PATCH v5 1/5] PCI: designware: ensure ATU is enabled before IO/conf space accesses
@ 2016-01-07  6:33       ` Jisheng Zhang
  0 siblings, 0 replies; 38+ messages in thread
From: Jisheng Zhang @ 2016-01-07  6:33 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Bjorn,

On Wed, 6 Jan 2016 12:20:03 -0600 Bjorn Helgaas wrote:

> [+cc Jisheng]
> 
> On Fri, Dec 18, 2015 at 02:38:55PM +0200, Stanimir Varbanov wrote:
> > There is no guarantees that enabling ATU will hit the hardware
> > immediately, and subsequent accesses to configuration / IO spaces
> > are reliable. So fixing this by read back PCIE_ATU_CR2 register
> > just after writing.
> > 
> > Without such a fix the 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 <stanimir.varbanov@linaro.org>
> > ---
> >  drivers/pci/host/pcie-designware.c |    7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> > index 02a7452bdf23..7880de63895d 100644
> > --- a/drivers/pci/host/pcie-designware.c
> > +++ b/drivers/pci/host/pcie-designware.c
> > @@ -154,6 +154,8 @@ static int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
> >  static void dw_pcie_prog_outbound_atu(struct pcie_port *pp, int index,
> >  		int type, u64 cpu_addr, u64 pci_addr, u32 size)
> >  {
> > +	u32 val;
> > +
> >  	dw_pcie_writel_rc(pp, PCIE_ATU_REGION_OUTBOUND | index,
> >  			  PCIE_ATU_VIEWPORT);
> >  	dw_pcie_writel_rc(pp, lower_32_bits(cpu_addr), PCIE_ATU_LOWER_BASE);
> > @@ -164,6 +166,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].
> > +	 */
> > +	dw_pcie_readl_rc(pp, PCIE_ATU_CR2, &val);  
> 
> This particular fix makes sense to me.
> 
> But I have a larger question about how the ATU works.  I see these
> definitions:
> 
>   #define PCIE_ATU_TYPE_MEM
>   #define PCIE_ATU_TYPE_IO
>   #define PCIE_ATU_TYPE_CFG0
>   #define PCIE_ATU_TYPE_CFG1
> 
> and these uses:
> 
>   - In dw_pcie_host_init(), set PCIE_ATU_TYPE_MEM for unit 1
>     (but only if rd_other_conf is not overridden)
> 
>   - In dw_pcie_rd_other_conf() and dw_pcie_wr_other_conf(),
>     set PCIE_ATU_TYPE_CFG0 before config access to own bus;
>     set PCIE_ATU_TYPE_CFG1 before config access to other bus;
>     set PCIE_ATU_TYPE_IO after completion
> 
> I'm confused:
> 
> 1) I assume PCIE_ATU_TYPE_MEM is for access to PCI memory space.  Why
> is that initialization related to rd_other_conf?  Shouldn't that be
> set up always?  A comment here would be nice, to clarify that this is
> not related to the subsequent dw_pcie_wr_own_conf() calls.

Indeed, the comment is necessary. I forget the reason until read the code
carefully again. The reason here is

If the platform provides ->rd_other_conf, it means the platform
doesn't support ATU, it uses its own address translation component
rather than ATU, so we should ignore ATU programming for this
kind of platform.

I have sent out one patch to add this comment.

> 
> 2) Why doesn't dw_pcie_host_init() use dw_pcie_rd_conf() instead of
> dw_pcie_rd_own_conf()?  Using pci_read_config_dword() might be even
> better.  Using the internal interfaces piecemeal like we do today is
> just an opportunity for doing it wrong.

IMHO, the reason is during host_init, the pci bus etc. are not initialized,
from another side, the code is really accessing its own conf registers.

> 
> 3) The definitions and your comment about "accessing PCI
> configuration/io spaces" suggest that the ATU must be programmed
> differently for accesses to PCI config space vs. I/O space.  If that's

Yes.

> the case, we'd need some kind of mutex to protect inl(), etc., during
> config accesses.  For example, in this scenario:
> 
>   thread A                                         thread B
>   ---------------------------------------------    ----------
>   pci_read_config_dword()
>   dw_pcie_prog_outbound_atu(PCIE_ATU_TYPE_CFG0)
>                                                    inl()
>   dw_pcie_cfg_read()
>   readl()
>   dw_pcie_prog_outbound_atu(PCIE_ATU_TYPE_IO)
>                                                    inl()
> 
> Do both inl() calls by thread B work correctly, even though the ATU
> seems to be programmed for CFG0 for the first but IO for the second?
> 

Indeed, there's such race issue as you and RMK pointed out.

IMHO, we need support:

1. platforms which contain three or more iATUs, there's no need to mux
iATU usage: one ATU for IO, one for cfg and another for MEM. But how to
get the number of iATUs, DT? eg. "snps,atu_num = 3"

2. platforms which contain only two iATUs, add mechanism to protect
the cfg vs IO race. Could you please give suggestions to how to achieve
this goal?

Thanks,
Jisheng

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

end of thread, other threads:[~2016-01-07  6:37 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-18 12:38 [PATCH v5 0/5] Qualcomm PCIe driver and designware fixes Stanimir Varbanov
2015-12-18 12:38 ` Stanimir Varbanov
2015-12-18 12:38 ` [PATCH v5 1/5] PCI: designware: ensure ATU is enabled before IO/conf space accesses Stanimir Varbanov
2015-12-18 12:38   ` Stanimir Varbanov
2015-12-18 14:41   ` Pratyush Anand
2015-12-18 14:41     ` Pratyush Anand
2015-12-18 14:41     ` Pratyush Anand
2016-01-04 14:31     ` Stanimir Varbanov
2016-01-04 14:31       ` Stanimir Varbanov
2016-01-04 14:31       ` Stanimir Varbanov
2016-01-06 18:20   ` Bjorn Helgaas
2016-01-06 18:20     ` Bjorn Helgaas
2016-01-07  6:33     ` Jisheng Zhang
2016-01-07  6:33       ` Jisheng Zhang
2016-01-07  6:33       ` Jisheng Zhang
2015-12-18 12:38 ` [PATCH v5 2/5] DT: PCI: qcom: Document PCIe devicetree bindings Stanimir Varbanov
2015-12-18 12:38   ` Stanimir Varbanov
2015-12-18 12:38 ` [PATCH v5 3/5] PCI: qcom: Add Qualcomm PCIe controller driver Stanimir Varbanov
2015-12-18 12:38   ` Stanimir Varbanov
2015-12-18 12:38   ` Stanimir Varbanov
2015-12-18 13:44   ` [PATCH] PCI: qcom: fix ptr_ret.cocci warnings kbuild test robot
2015-12-18 13:44     ` kbuild test robot
2015-12-18 13:44     ` kbuild test robot
2015-12-18 13:44   ` [PATCH v5 3/5] PCI: qcom: Add Qualcomm PCIe controller driver kbuild test robot
2015-12-18 13:44     ` kbuild test robot
2015-12-18 13:44     ` kbuild test robot
2015-12-20 23:10     ` Bjorn Andersson
2015-12-20 23:10       ` Bjorn Andersson
2015-12-20 23:10       ` Bjorn Andersson
2015-12-21 23:04       ` Arnd Bergmann
2015-12-21 23:04         ` Arnd Bergmann
2015-12-21 23:04         ` Arnd Bergmann
2015-12-18 12:38 ` [PATCH v5 4/5] ARM: dts: apq8064: add pcie devicetree node Stanimir Varbanov
2015-12-18 12:38   ` Stanimir Varbanov
2015-12-18 12:38 ` [PATCH v5 5/5] ARM: dts: ifc6410: enable pcie dt node for this board Stanimir Varbanov
2015-12-18 12:38   ` Stanimir Varbanov
2016-01-05 21:42 ` [PATCH v5 0/5] Qualcomm PCIe driver and designware fixes Bjorn Helgaas
2016-01-05 21:42   ` Bjorn Helgaas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.