linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/11] Add support for Hikey 970 PCIe
@ 2021-02-03  7:01 Mauro Carvalho Chehab
  2021-02-03  7:01 ` [PATCH v2 01/11] doc: bindings: PCI: designware-pcie.txt: convert it to YAML Mauro Carvalho Chehab
                   ` (10 more replies)
  0 siblings, 11 replies; 24+ messages in thread
From: Mauro Carvalho Chehab @ 2021-02-03  7:01 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Andy Gross, Binghui Wang, Bjorn Andersson,
	Bjorn Helgaas, Fabio Estevam, Gustavo Pimentel, Jaehoon Chung,
	Jerome Brunet, Jesper Nilsson, Jingoo Han, Jonathan Chocron,
	Jonathan Hunter, Kevin Hilman, Kishon Vijay Abraham I,
	Krzysztof Kozlowski, Kunihiko Hayashi, Liam Girdwood,
	Lorenzo Pieralisi, Lucas Stach, Marek Szyprowski, Mark Brown,
	Martin Blumenstingl, NXP Linux Team, Neil Armstrong,
	Pengutronix Kernel Team, Richard Zhu, Rob Herring, Rob Herring,
	Sascha Hauer, Shawn Guo, Thierry Reding, Thomas Petazzoni,
	Xiaowei Song, Zhou Wang, devicetree, linux-amlogic,
	linux-arm-kernel, linux-arm-kernel, linux-arm-msm, linux-kernel,
	linux-omap, linux-pci, linux-samsung-soc, linux-tegra

This series add PCIe support for Kirin 970 SoC.

Patch 1 converts the Synopsys DesignWare PCIe binding documentation to
the DT schema;

Patch 2 converts the pcie-kirin DT binding to the DT schema;

Patch 3 adds some extra configuration needed to support Kirin 970.

Patch 4 were imported from Manivannan's HiKey 970 96boards tree:

   https://git.linaro.org/people/manivannan.sadhasivam/96b-common.git/commit/?h=hikey970_pcie&id=4917380ad023c62960aa0f876bd4f23cefc8729e

It contains the original port made by Linaro.

The remaining patches contain several cleanups applied on the top of
Manivann's work.

They cleanup the code and change it to use the DT bindings defined on
patch 3.

---

v2:
- DTS bindings dropped, as they depend on other DTS changes that will be
   happening at staging and ARM trees;
- Use regulator_get() for Kirin 970, instead of regulator_get_optional();
- The power supply was renamed to "pcie_vdd", in order to better match the
  schematics;
- The patch descriptions were renamed in order to match the terms used
  by other PCI patches;
- dts patches removed from this series, as they depend on other patches
  being merged via other trees.

Manivannan Sadhasivam (1):
  PCI: dwc: pcie-kirin: add support for Kirin 970 PCIe controller

Mauro Carvalho Chehab (10):
  doc: bindings: PCI: designware-pcie.txt: convert it to YAML
  doc: bindings: kirin-pcie.txt: convert it to YAML
  doc: bindings: add new parameters used by Kirin 970
  PCI: dwc: pcie-kirin: simplify error handling logic
  PCI: dwc: pcie-kirin: simplify Kirin 970 get resource logic
  PCI: dwc: pcie-kirin: place common init code altogether
  PCI: dwc: pcie-kirin: add support for a regulator
  PCI: dwc: pcie-kirin: allow using multiple reset GPIOs
  PCI: dwc: pcie-kirin: add support for clkreq GPIOs
  pci: dwc: pcie-kirin: cleanup kirin970_pcie_get_eyeparam()

 .../bindings/pci/amlogic,meson-pcie.txt       |   4 +-
 .../bindings/pci/axis,artpec6-pcie.txt        |   2 +-
 .../bindings/pci/designware-pcie.txt          |  77 --
 .../bindings/pci/fsl,imx6q-pcie.txt           |   2 +-
 .../bindings/pci/hisilicon,kirin-pcie.yaml    | 144 ++++
 .../bindings/pci/hisilicon-histb-pcie.txt     |   2 +-
 .../bindings/pci/hisilicon-pcie.txt           |   2 +-
 .../devicetree/bindings/pci/kirin-pcie.txt    |  50 --
 .../bindings/pci/layerscape-pci.txt           |   2 +-
 .../bindings/pci/nvidia,tegra194-pcie.txt     |   4 +-
 .../devicetree/bindings/pci/pci-armada8k.txt  |   2 +-
 .../devicetree/bindings/pci/pci-keystone.txt  |  10 +-
 .../devicetree/bindings/pci/pcie-al.txt       |   2 +-
 .../devicetree/bindings/pci/qcom,pcie.txt     |  14 +-
 .../bindings/pci/samsung,exynos-pcie.yaml     |   2 +-
 .../devicetree/bindings/pci/snps,pcie.yaml    | 139 ++++
 .../pci/socionext,uniphier-pcie-ep.yaml       |   2 +-
 .../devicetree/bindings/pci/ti-pci.txt        |   4 +-
 .../devicetree/bindings/pci/uniphier-pcie.txt |   2 +-
 MAINTAINERS                                   |   4 +-
 drivers/pci/controller/dwc/pcie-kirin.c       | 749 +++++++++++++++++-
 21 files changed, 1033 insertions(+), 186 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/pci/designware-pcie.txt
 create mode 100644 Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
 delete mode 100644 Documentation/devicetree/bindings/pci/kirin-pcie.txt
 create mode 100644 Documentation/devicetree/bindings/pci/snps,pcie.yaml

-- 
2.29.2



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

* [PATCH v2 01/11] doc: bindings: PCI: designware-pcie.txt: convert it to YAML
  2021-02-03  7:01 [PATCH v2 00/11] Add support for Hikey 970 PCIe Mauro Carvalho Chehab
@ 2021-02-03  7:01 ` Mauro Carvalho Chehab
  2021-02-04 15:20   ` Rob Herring
  2021-02-03  7:01 ` [PATCH v2 02/11] doc: bindings: kirin-pcie.txt: " Mauro Carvalho Chehab
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 24+ messages in thread
From: Mauro Carvalho Chehab @ 2021-02-03  7:01 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Andy Gross, Bjorn Andersson,
	Bjorn Helgaas, Fabio Estevam, Gustavo Pimentel, Jaehoon Chung,
	Jerome Brunet, Jesper Nilsson, Jingoo Han, Jonathan Chocron,
	Jonathan Hunter, Kevin Hilman, Kishon Vijay Abraham I,
	Krzysztof Kozlowski, Kunihiko Hayashi, Lucas Stach,
	Marek Szyprowski, Martin Blumenstingl, NXP Linux Team,
	Neil Armstrong, Pengutronix Kernel Team, Richard Zhu,
	Rob Herring, Sascha Hauer, Shawn Guo, Thierry Reding,
	Thomas Petazzoni, Zhou Wang, devicetree, linux-amlogic,
	linux-arm-kernel, linux-arm-kernel, linux-arm-msm, linux-kernel,
	linux-omap, linux-pci, linux-samsung-soc, linux-tegra

Convert the file into a DT schema.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 .../bindings/pci/amlogic,meson-pcie.txt       |   4 +-
 .../bindings/pci/axis,artpec6-pcie.txt        |   2 +-
 .../bindings/pci/designware-pcie.txt          |  77 ----------
 .../bindings/pci/fsl,imx6q-pcie.txt           |   2 +-
 .../bindings/pci/hisilicon-histb-pcie.txt     |   2 +-
 .../bindings/pci/hisilicon-pcie.txt           |   2 +-
 .../devicetree/bindings/pci/kirin-pcie.txt    |   2 +-
 .../bindings/pci/layerscape-pci.txt           |   2 +-
 .../bindings/pci/nvidia,tegra194-pcie.txt     |   4 +-
 .../devicetree/bindings/pci/pci-armada8k.txt  |   2 +-
 .../devicetree/bindings/pci/pci-keystone.txt  |  10 +-
 .../devicetree/bindings/pci/pcie-al.txt       |   2 +-
 .../devicetree/bindings/pci/qcom,pcie.txt     |  14 +-
 .../bindings/pci/samsung,exynos-pcie.yaml     |   2 +-
 .../devicetree/bindings/pci/snps,pcie.yaml    | 139 ++++++++++++++++++
 .../pci/socionext,uniphier-pcie-ep.yaml       |   2 +-
 .../devicetree/bindings/pci/ti-pci.txt        |   4 +-
 .../devicetree/bindings/pci/uniphier-pcie.txt |   2 +-
 MAINTAINERS                                   |   2 +-
 19 files changed, 169 insertions(+), 107 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/pci/designware-pcie.txt
 create mode 100644 Documentation/devicetree/bindings/pci/snps,pcie.yaml

diff --git a/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt b/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt
index b6acbe694ffb..da9253d43550 100644
--- a/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt
@@ -3,7 +3,7 @@ Amlogic Meson AXG DWC PCIE SoC controller
 Amlogic Meson PCIe host controller is based on the Synopsys DesignWare PCI core.
 It shares common functions with the PCIe DesignWare core driver and
 inherits common properties defined in
-Documentation/devicetree/bindings/pci/designware-pcie.txt.
+Documentation/devicetree/bindings/pci/snps,pcie.yaml.
 
 Additional properties are described here:
 
@@ -33,7 +33,7 @@ Required properties:
 - phy-names: must contain "pcie"
 
 - device_type:
-	should be "pci". As specified in designware-pcie.txt
+	should be "pci". As specified in snps,pcie.yaml
 
 
 Example configuration:
diff --git a/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt b/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
index 979dc7b6cfe8..84b53ae2c376 100644
--- a/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
@@ -1,7 +1,7 @@
 * Axis ARTPEC-6 PCIe interface
 
 This PCIe host controller is based on the Synopsys DesignWare PCIe IP
-and thus inherits all the common properties defined in designware-pcie.txt.
+and thus inherits all the common properties defined in snps,pcie.yaml.
 
 Required properties:
 - compatible: "axis,artpec6-pcie", "snps,dw-pcie" for ARTPEC-6 in RC mode;
diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
deleted file mode 100644
index 78494c4050f7..000000000000
--- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
+++ /dev/null
@@ -1,77 +0,0 @@
-* Synopsys DesignWare PCIe interface
-
-Required properties:
-- compatible:
-	"snps,dw-pcie" for RC mode;
-	"snps,dw-pcie-ep" for EP mode;
-- reg: For designware cores version < 4.80 contains the configuration
-       address space. For designware core version >= 4.80, contains
-       the configuration and ATU address space
-- reg-names: Must be "config" for the PCIe configuration space and "atu" for
-	     the ATU address space.
-    (The old way of getting the configuration address space from "ranges"
-    is deprecated and should be avoided.)
-RC mode:
-- #address-cells: set to <3>
-- #size-cells: set to <2>
-- device_type: set to "pci"
-- ranges: ranges for the PCI memory and I/O regions
-- #interrupt-cells: set to <1>
-- interrupt-map-mask and interrupt-map: standard PCI
-	properties to define the mapping of the PCIe interface to interrupt
-	numbers.
-EP mode:
-- num-ib-windows: number of inbound address translation windows
-- num-ob-windows: number of outbound address translation windows
-
-Optional properties:
-- num-lanes: number of lanes to use (this property should be specified unless
-  the link is brought already up in BIOS)
-- reset-gpio: GPIO pin number of power good signal
-- clocks: Must contain an entry for each entry in clock-names.
-	See ../clocks/clock-bindings.txt for details.
-- clock-names: Must include the following entries:
-	- "pcie"
-	- "pcie_bus"
-- snps,enable-cdm-check: This is a boolean property and if present enables
-   automatic checking of CDM (Configuration Dependent Module) registers
-   for data corruption. CDM registers include standard PCIe configuration
-   space registers, Port Logic registers, DMA and iATU (internal Address
-   Translation Unit) registers.
-RC mode:
-- num-viewport: number of view ports configured in hardware. If a platform
-  does not specify it, the driver assumes 2.
-- bus-range: PCI bus numbers covered (it is recommended for new devicetrees
-  to specify this property, to keep backwards compatibility a range of
-  0x00-0xff is assumed if not present)
-
-EP mode:
-- max-functions: maximum number of functions that can be configured
-
-Example configuration:
-
-	pcie: pcie@dfc00000 {
-		compatible = "snps,dw-pcie";
-		reg = <0xdfc00000 0x0001000>, /* IP registers */
-		      <0xd0000000 0x0002000>; /* Configuration space */
-		reg-names = "dbi", "config";
-		#address-cells = <3>;
-		#size-cells = <2>;
-		device_type = "pci";
-		ranges = <0x81000000 0 0x00000000 0xde000000 0 0x00010000
-			  0x82000000 0 0xd0400000 0xd0400000 0 0x0d000000>;
-		interrupts = <25>, <24>;
-		#interrupt-cells = <1>;
-		num-lanes = <1>;
-	};
-or
-	pcie: pcie@dfc00000 {
-		compatible = "snps,dw-pcie-ep";
-		reg = <0xdfc00000 0x0001000>, /* IP registers 1 */
-		      <0xdfc01000 0x0001000>, /* IP registers 2 */
-		      <0xd0000000 0x2000000>; /* Configuration space */
-		reg-names = "dbi", "dbi2", "addr_space";
-		num-ib-windows = <6>;
-		num-ob-windows = <2>;
-		num-lanes = <1>;
-	};
diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
index de4b2baf91e8..9470b279e3e4 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
@@ -1,7 +1,7 @@
 * Freescale i.MX6 PCIe interface
 
 This PCIe host controller is based on the Synopsys DesignWare PCIe IP
-and thus inherits all the common properties defined in designware-pcie.txt.
+and thus inherits all the common properties defined in snps,pcie.yaml.
 
 Required properties:
 - compatible:
diff --git a/Documentation/devicetree/bindings/pci/hisilicon-histb-pcie.txt b/Documentation/devicetree/bindings/pci/hisilicon-histb-pcie.txt
index 760b4d740616..2f62119d97b9 100644
--- a/Documentation/devicetree/bindings/pci/hisilicon-histb-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/hisilicon-histb-pcie.txt
@@ -3,7 +3,7 @@ HiSilicon STB PCIe host bridge DT description
 The HiSilicon STB PCIe host controller is based on the DesignWare PCIe core.
 It shares common functions with the DesignWare PCIe core driver and inherits
 common properties defined in
-Documentation/devicetree/bindings/pci/designware-pcie.txt.
+Documentation/devicetree/bindings/pci/snps,pcie.yaml.
 
 Additional properties are described here:
 
diff --git a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
index d6796ef54ea1..4d243bfb9709 100644
--- a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
@@ -3,7 +3,7 @@ HiSilicon Hip05 and Hip06 PCIe host bridge DT description
 HiSilicon PCIe host controller is based on the Synopsys DesignWare PCI core.
 It shares common functions with the PCIe DesignWare core driver and inherits
 common properties defined in
-Documentation/devicetree/bindings/pci/designware-pcie.txt.
+Documentation/devicetree/bindings/pci/snps,pcie.yaml.
 
 Additional properties are described here:
 
diff --git a/Documentation/devicetree/bindings/pci/kirin-pcie.txt b/Documentation/devicetree/bindings/pci/kirin-pcie.txt
index 6bbe43818ad5..a38f8e38a67b 100644
--- a/Documentation/devicetree/bindings/pci/kirin-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/kirin-pcie.txt
@@ -3,7 +3,7 @@ HiSilicon Kirin SoCs PCIe host DT description
 Kirin PCIe host controller is based on the Synopsys DesignWare PCI core.
 It shares common functions with the PCIe DesignWare core driver and
 inherits common properties defined in
-Documentation/devicetree/bindings/pci/designware-pcie.txt.
+Documentation/devicetree/bindings/pci/snps,pcie.yaml.
 
 Additional properties are described here:
 
diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
index daa99f7d4c3f..8070adfc1746 100644
--- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
+++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
@@ -1,7 +1,7 @@
 Freescale Layerscape PCIe controller
 
 This PCIe host controller is based on the Synopsys DesignWare PCIe IP
-and thus inherits all the common properties defined in designware-pcie.txt.
+and thus inherits all the common properties defined in snps,pcie.yaml.
 
 This controller derives its clocks from the Reset Configuration Word (RCW)
 which is used to describe the PLL settings at the time of chip-reset.
diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.txt b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.txt
index bd43f3c3ece4..4644d79e0e0c 100644
--- a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.txt
@@ -1,7 +1,7 @@
 NVIDIA Tegra PCIe controller (Synopsys DesignWare Core based)
 
 This PCIe controller is based on the Synopsis Designware PCIe IP
-and thus inherits all the common properties defined in designware-pcie.txt.
+and thus inherits all the common properties defined in snps,pcie.yaml.
 Some of the controller instances are dual mode where in they can work either
 in root port mode or endpoint mode but one at a time.
 
@@ -22,7 +22,7 @@ Required properties:
   property.
 - reg-names: Must include the following entries:
   "appl": Controller's application logic registers
-  "config": As per the definition in designware-pcie.txt
+  "config": As per the definition in snps,pcie.yaml
   "atu_dma": iATU and DMA registers. This is where the iATU (internal Address
              Translation Unit) registers of the PCIe core are made available
              for SW access.
diff --git a/Documentation/devicetree/bindings/pci/pci-armada8k.txt b/Documentation/devicetree/bindings/pci/pci-armada8k.txt
index 7a813d0e6d63..4531b895e9ea 100644
--- a/Documentation/devicetree/bindings/pci/pci-armada8k.txt
+++ b/Documentation/devicetree/bindings/pci/pci-armada8k.txt
@@ -1,7 +1,7 @@
 * Marvell Armada 7K/8K PCIe interface
 
 This PCIe host controller is based on the Synopsys DesignWare PCIe IP
-and thus inherits all the common properties defined in designware-pcie.txt.
+and thus inherits all the common properties defined in snps,pcie.yaml.
 
 Required properties:
 - compatible: "marvell,armada8k-pcie"
diff --git a/Documentation/devicetree/bindings/pci/pci-keystone.txt b/Documentation/devicetree/bindings/pci/pci-keystone.txt
index 47202a2938f2..ac271a0ca078 100644
--- a/Documentation/devicetree/bindings/pci/pci-keystone.txt
+++ b/Documentation/devicetree/bindings/pci/pci-keystone.txt
@@ -3,9 +3,9 @@ TI Keystone PCIe interface
 Keystone PCI host Controller is based on the Synopsys DesignWare PCI
 hardware version 3.65.  It shares common functions with the PCIe DesignWare
 core driver and inherits common properties defined in
-Documentation/devicetree/bindings/pci/designware-pcie.txt
+Documentation/devicetree/bindings/pci/snps,pcie.yaml
 
-Please refer to Documentation/devicetree/bindings/pci/designware-pcie.txt
+Please refer to Documentation/devicetree/bindings/pci/snps,pcie.yaml
 for the details of DesignWare DT bindings.  Additional properties are
 described here as well as properties that are not applicable.
 
@@ -82,11 +82,11 @@ reg-names: "dbics" for the DesignWare PCIe registers, "app" for the
 	   Address Translation Unit configuration registers and
 	   "addr_space" used to map remote RC address space
 num-ib-windows: As specified in
-		Documentation/devicetree/bindings/pci/designware-pcie.txt
+		Documentation/devicetree/bindings/pci/snps,pcie.yaml
 num-ob-windows: As specified in
-		Documentation/devicetree/bindings/pci/designware-pcie.txt
+		Documentation/devicetree/bindings/pci/snps,pcie.yaml
 num-lanes: As specified in
-	   Documentation/devicetree/bindings/pci/designware-pcie.txt
+	   Documentation/devicetree/bindings/pci/snps,pcie.yaml
 power-domains: As documented by the generic PM domain bindings in
 	       Documentation/devicetree/bindings/power/power_domain.txt.
 ti,syscon-pcie-mode: phandle to the device control module required to configure
diff --git a/Documentation/devicetree/bindings/pci/pcie-al.txt b/Documentation/devicetree/bindings/pci/pcie-al.txt
index 557a5089229d..6e1e20e15ae9 100644
--- a/Documentation/devicetree/bindings/pci/pcie-al.txt
+++ b/Documentation/devicetree/bindings/pci/pcie-al.txt
@@ -2,7 +2,7 @@
 
 Amazon's Annapurna Labs PCIe Host Controller is based on the Synopsys DesignWare
 PCI core. It inherits common properties defined in
-Documentation/devicetree/bindings/pci/designware-pcie.txt.
+Documentation/devicetree/bindings/pci/snps,pcie.yaml.
 
 Properties of the host controller node that differ from it are:
 
diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
index 3b55310390a0..43c3086cf6ea 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
@@ -33,22 +33,22 @@
 - device_type:
 	Usage: required
 	Value type: <string>
-	Definition: Should be "pci". As specified in designware-pcie.txt
+	Definition: Should be "pci". As specified in snps,pcie.yaml
 
 - #address-cells:
 	Usage: required
 	Value type: <u32>
-	Definition: Should be 3. As specified in designware-pcie.txt
+	Definition: Should be 3. As specified in snps,pcie.yaml
 
 - #size-cells:
 	Usage: required
 	Value type: <u32>
-	Definition: Should be 2. As specified in designware-pcie.txt
+	Definition: Should be 2. As specified in snps,pcie.yaml
 
 - ranges:
 	Usage: required
 	Value type: <prop-encoded-array>
-	Definition: As specified in designware-pcie.txt
+	Definition: As specified in snps,pcie.yaml
 
 - interrupts:
 	Usage: required
@@ -63,17 +63,17 @@
 - #interrupt-cells:
 	Usage: required
 	Value type: <u32>
-	Definition: Should be 1. As specified in designware-pcie.txt
+	Definition: Should be 1. As specified in snps,pcie.yaml
 
 - interrupt-map-mask:
 	Usage: required
 	Value type: <prop-encoded-array>
-	Definition: As specified in designware-pcie.txt
+	Definition: As specified in snps,pcie.yaml
 
 - interrupt-map:
 	Usage: required
 	Value type: <prop-encoded-array>
-	Definition: As specified in designware-pcie.txt
+	Definition: As specified in snps,pcie.yaml
 
 - clocks:
 	Usage: required
diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
index 1810bf722350..8a4fe2d021ed 100644
--- a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml
@@ -13,7 +13,7 @@ maintainers:
 description: |+
   Exynos5433 SoC PCIe host controller is based on the Synopsys DesignWare
   PCIe IP and thus inherits all the common properties defined in
-  designware-pcie.txt.
+  snps,pcie.yaml.
 
 allOf:
   - $ref: /schemas/pci/pci-bus.yaml#
diff --git a/Documentation/devicetree/bindings/pci/snps,pcie.yaml b/Documentation/devicetree/bindings/pci/snps,pcie.yaml
new file mode 100644
index 000000000000..eeb5dad6937d
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/snps,pcie.yaml
@@ -0,0 +1,139 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/snps,pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys DesignWare PCIe interface
+
+maintainers:
+  - Jingoo Han <jingoohan1@gmail.com>
+  - Gustavo Pimentel <gustavo.pimentel@synopsys.com>
+
+description: |
+  Synopsys DesignWare PCIe host controller
+
+anyOf:
+  - {}
+  - items:
+      contains:
+        enum:
+          - snps,dw-pcie
+          - snps,dw-pcie-ep
+
+properties:
+  compatible:
+    description: |
+      The compatible can be either:
+      - snps,dw-pcie       # for RC mode
+      - snps,dw-pcie-ep    # For EP mode
+      or some other value, when there's a host-specific driver
+
+  reg:
+    description: |
+      Contains DBI and the configuration address space for all
+      Designware versions.
+      For Designware core version >= 4.80, should also contain the
+      ATU address space.
+    minItems: 2
+    maxItems: 4
+
+  reset-gpio:
+    description: GPIO pin number for the PERST# signal
+    maxItems: 1
+
+  clocks:
+    minItems: 2
+    maxItems: 8
+
+  clock-names:
+    items:
+      contains:
+        enum: [ pcie, pcie_bus ]
+    minItems: 2
+    maxItems: 8
+
+  "snps,enable-cdm-check":
+    $ref: /schemas/types.yaml#definitions/flag
+    description: |
+      This is a boolean property and if present enables
+      automatic checking of CDM (Configuration Dependent Module) registers
+      for data corruption. CDM registers include standard PCIe configuration
+      space registers, Port Logic registers, DMA and iATU (internal Address
+      Translation Unit) registers.
+
+  # The following are optional properties for RC mode
+
+  num-viewport:
+    description: |
+      number of view ports configured in hardware. If a platform
+      does not specify it, the driver assumes 2.
+    deprecated: true
+
+  # The following are mandatory properties for EP Mode
+
+  num-ib-windows:
+    description: number of inbound address translation windows
+    maxItems: 1
+    deprecated: true
+
+  num-ob-windows:
+    description: number of outbound address translation windows
+    maxItems: 1
+    deprecated: true
+
+  # The following are optional properties for EP mode
+
+  max-functions:
+    description: maximum number of functions that can be configured
+
+required:
+  - reg
+  - reg-names
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: snps,dw-pcie
+    then:
+      allOf:
+        - $ref: /schemas/pci/pci-bus.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: snps,dw-pcie-ep
+    then:
+      required:
+        - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    pcie: pcie@dfc00000 {
+      compatible = "snps,dw-pcie";
+      reg = <0xdfc00000 0x0001000>, /* IP registers */
+            <0xd0000000 0x0002000>; /* Configuration space */
+      reg-names = "dbi", "config";
+      #address-cells = <3>;
+      #size-cells = <2>;
+      device_type = "pci";
+      ranges = <0x81000000 0 0x00000000 0xde000000 0 0x00010000>,
+               <0x82000000 0 0xd0400000 0xd0400000 0 0x0d000000>;
+      interrupts = <25>, <24>;
+      #interrupt-cells = <1>;
+      num-lanes = <1>;
+    };
+    pcie_ep: pcie_ep@dfd00000 {
+      compatible = "snps,dw-pcie-ep";
+      reg = <0xdfc00000 0x0001000>, /* IP registers 1 */
+            <0xdfc01000 0x0001000>, /* IP registers 2 */
+            <0xd0000000 0x2000000>; /* Configuration space */
+      reg-names = "dbi", "dbi2", "addr_space";
+      num-ib-windows = <6>;
+      num-ob-windows = <2>;
+      num-lanes = <1>;
+    };
diff --git a/Documentation/devicetree/bindings/pci/socionext,uniphier-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/socionext,uniphier-pcie-ep.yaml
index d6cf8a560ef0..9c8733d21de4 100644
--- a/Documentation/devicetree/bindings/pci/socionext,uniphier-pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/socionext,uniphier-pcie-ep.yaml
@@ -10,7 +10,7 @@ description: |
   UniPhier PCIe endpoint controller is based on the Synopsys DesignWare
   PCI core. It shares common features with the PCIe DesignWare core and
   inherits common properties defined in
-  Documentation/devicetree/bindings/pci/designware-pcie.txt.
+  Documentation/devicetree/bindings/pci/snps,pcie.yaml.
 
 maintainers:
   - Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
diff --git a/Documentation/devicetree/bindings/pci/ti-pci.txt b/Documentation/devicetree/bindings/pci/ti-pci.txt
index d5cbfe6b0d89..dfebfdb9b819 100644
--- a/Documentation/devicetree/bindings/pci/ti-pci.txt
+++ b/Documentation/devicetree/bindings/pci/ti-pci.txt
@@ -12,7 +12,7 @@ PCIe DesignWare Controller
 	       number of PHYs as specified in *phys* property.
  - ti,hwmods : Name of the hwmod associated to the pcie, "pcie<X>",
 	       where <X> is the instance number of the pcie from the HW spec.
- - num-lanes as specified in ../designware-pcie.txt
+ - num-lanes as specified in ../snps,pcie.yaml
  - ti,syscon-lane-sel : phandle/offset pair. Phandle to the system control
 			module and the register offset to specify lane
 			selection.
@@ -32,7 +32,7 @@ HOST MODE
    device_type,
    ranges,
    interrupt-map-mask,
-   interrupt-map : as specified in ../designware-pcie.txt
+   interrupt-map : as specified in ../snps,pcie.yaml
  - ti,syscon-unaligned-access: phandle to the syscon DT node. The 1st argument
 			       should contain the register offset within syscon
 			       and the 2nd argument should contain the bit field
diff --git a/Documentation/devicetree/bindings/pci/uniphier-pcie.txt b/Documentation/devicetree/bindings/pci/uniphier-pcie.txt
index c4b7381733a0..17cfe504d800 100644
--- a/Documentation/devicetree/bindings/pci/uniphier-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/uniphier-pcie.txt
@@ -6,7 +6,7 @@ on Socionext UniPhier SoCs.
 UniPhier PCIe host controller is based on the Synopsys DesignWare PCI core.
 It shares common functions with the PCIe DesignWare core driver and inherits
 common properties defined in
-Documentation/devicetree/bindings/pci/designware-pcie.txt.
+Documentation/devicetree/bindings/pci/snps,pcie.yaml.
 
 Required properties:
 - compatible: Should be "socionext,uniphier-pcie".
diff --git a/MAINTAINERS b/MAINTAINERS
index 2dd0a662d931..0bcba0d4994c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13673,7 +13673,7 @@ M:	Jingoo Han <jingoohan1@gmail.com>
 M:	Gustavo Pimentel <gustavo.pimentel@synopsys.com>
 L:	linux-pci@vger.kernel.org
 S:	Maintained
-F:	Documentation/devicetree/bindings/pci/designware-pcie.txt
+F:	Documentation/devicetree/bindings/pci/snps,pcie.yaml
 F:	drivers/pci/controller/dwc/*designware*
 
 PCI DRIVER FOR TI DRA7XX/J721E
-- 
2.29.2


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

* [PATCH v2 02/11] doc: bindings: kirin-pcie.txt: convert it to YAML
  2021-02-03  7:01 [PATCH v2 00/11] Add support for Hikey 970 PCIe Mauro Carvalho Chehab
  2021-02-03  7:01 ` [PATCH v2 01/11] doc: bindings: PCI: designware-pcie.txt: convert it to YAML Mauro Carvalho Chehab
@ 2021-02-03  7:01 ` Mauro Carvalho Chehab
  2021-02-03  7:01 ` [PATCH v2 03/11] doc: bindings: add new parameters used by Kirin 970 Mauro Carvalho Chehab
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 24+ messages in thread
From: Mauro Carvalho Chehab @ 2021-02-03  7:01 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Binghui Wang, Bjorn Helgaas, Rob Herring,
	Xiaowei Song, devicetree, linux-kernel, linux-pci

Convert	the file into a	DT schema.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 .../bindings/pci/hisilicon,kirin-pcie.yaml    | 90 +++++++++++++++++++
 .../devicetree/bindings/pci/kirin-pcie.txt    | 50 -----------
 MAINTAINERS                                   |  2 +-
 3 files changed, 91 insertions(+), 51 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
 delete mode 100644 Documentation/devicetree/bindings/pci/kirin-pcie.txt

diff --git a/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
new file mode 100644
index 000000000000..46f9f3f25dbc
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
@@ -0,0 +1,90 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/hisilicon,kirin-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: HiSilicon Kirin SoCs PCIe host DT description
+
+maintainers:
+  - Xiaowei Song <songxiaowei@hisilicon.com>
+  - Binghui Wang <wangbinghui@hisilicon.com>
+
+description: |
+  Kirin PCIe host controller is based on the Synopsys DesignWare PCI core.
+  It shares common functions with the PCIe DesignWare core driver.
+
+allOf:
+  - $ref: /schemas/pci/pci-bus.yaml#
+#  - $ref: snps,pcie.yaml#
+
+properties:
+  compatible:
+    const: hisilicon,kirin960-pcie
+
+  reg:
+    description: |
+      Should contain rc_dbi, apb, phy, config registers location and length.
+
+  reg-names:
+    items:
+      - const: dbi          # controller configuration registers
+      - const: apb          # apb Ctrl register defined by Kirin
+      - const: phy          # apb PHY register defined by Kirin
+      - const: config       # PCIe configuration space registers
+
+  reset-gpios:
+    description: The GPIO to generate PCIe PERST# assert and deassert signal.
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - reset-gpios
+  - "#interrupt-cells"
+  - interrupt-map-mask
+  - interrupt-map
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/hi3660-clock.h>
+
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      pcie: pcie@f4000000 {
+        compatible = "hisilicon,kirin960-pcie";
+        reg = <0x0 0xf4000000 0x0 0x1000>,
+              <0x0 0xff3fe000 0x0 0x1000>,
+              <0x0 0xf3f20000 0x0 0x40000>,
+              <0x0 0xF4000000 0 0x2000>;
+        reg-names = "dbi","apb","phy", "config";
+        bus-range = <0x0  0x1>;
+        #address-cells = <3>;
+        #size-cells = <2>;
+        device_type = "pci";
+        ranges = <0x02000000 0x0 0x00000000 0x0 0xf5000000 0x0 0x2000000>;
+        num-lanes = <1>;
+        #interrupt-cells = <1>;
+        interrupts = <0 283 4>;
+        interrupt-names = "msi";
+        interrupt-map-mask = <0xf800 0 0 7>;
+        interrupt-map = <0x0 0 0 1 &gic GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>,
+                        <0x0 0 0 2 &gic GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
+                        <0x0 0 0 3 &gic GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
+                        <0x0 0 0 4 &gic GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&crg_ctrl HI3660_PCIEPHY_REF>,
+                 <&crg_ctrl HI3660_CLK_GATE_PCIEAUX>,
+                 <&crg_ctrl HI3660_PCLK_GATE_PCIE_PHY>,
+                 <&crg_ctrl HI3660_PCLK_GATE_PCIE_SYS>,
+                 <&crg_ctrl HI3660_ACLK_GATE_PCIE>;
+        clock-names = "pcie_phy_ref", "pcie_aux", "pcie_apb_phy",
+                      "pcie_apb_sys", "pcie_aclk";
+        reset-gpios = <&gpio11 1 0 >;
+      };
+    };
diff --git a/Documentation/devicetree/bindings/pci/kirin-pcie.txt b/Documentation/devicetree/bindings/pci/kirin-pcie.txt
deleted file mode 100644
index a38f8e38a67b..000000000000
--- a/Documentation/devicetree/bindings/pci/kirin-pcie.txt
+++ /dev/null
@@ -1,50 +0,0 @@
-HiSilicon Kirin SoCs PCIe host DT description
-
-Kirin PCIe host controller is based on the Synopsys DesignWare PCI core.
-It shares common functions with the PCIe DesignWare core driver and
-inherits common properties defined in
-Documentation/devicetree/bindings/pci/snps,pcie.yaml.
-
-Additional properties are described here:
-
-Required properties
-- compatible:
-	"hisilicon,kirin960-pcie" for PCIe of Kirin960 SoC
-- reg: Should contain rc_dbi, apb, phy, config registers location and length.
-- reg-names: Must include the following entries:
-  "dbi": controller configuration registers;
-  "apb": apb Ctrl register defined by Kirin;
-  "phy": apb PHY register defined by Kirin;
-  "config": PCIe configuration space registers.
-- reset-gpios: The GPIO to generate PCIe PERST# assert and deassert signal.
-
-Optional properties:
-
-Example based on kirin960:
-
-	pcie@f4000000 {
-		compatible = "hisilicon,kirin-pcie";
-		reg = <0x0 0xf4000000 0x0 0x1000>, <0x0 0xff3fe000 0x0 0x1000>,
-		      <0x0 0xf3f20000 0x0 0x40000>, <0x0 0xF4000000 0 0x2000>;
-		reg-names = "dbi","apb","phy", "config";
-		bus-range = <0x0  0x1>;
-		#address-cells = <3>;
-		#size-cells = <2>;
-		device_type = "pci";
-		ranges = <0x02000000 0x0 0x00000000 0x0 0xf5000000 0x0 0x2000000>;
-		num-lanes = <1>;
-		#interrupt-cells = <1>;
-		interrupt-map-mask = <0xf800 0 0 7>;
-		interrupt-map = <0x0 0 0 1 &gic 0 0 0  282 4>,
-				<0x0 0 0 2 &gic 0 0 0  283 4>,
-				<0x0 0 0 3 &gic 0 0 0  284 4>,
-				<0x0 0 0 4 &gic 0 0 0  285 4>;
-		clocks = <&crg_ctrl HI3660_PCIEPHY_REF>,
-			 <&crg_ctrl HI3660_CLK_GATE_PCIEAUX>,
-			 <&crg_ctrl HI3660_PCLK_GATE_PCIE_PHY>,
-			 <&crg_ctrl HI3660_PCLK_GATE_PCIE_SYS>,
-			 <&crg_ctrl HI3660_ACLK_GATE_PCIE>;
-		clock-names = "pcie_phy_ref", "pcie_aux",
-			      "pcie_apb_phy", "pcie_apb_sys", "pcie_aclk";
-		reset-gpios = <&gpio11 1 0 >;
-	};
diff --git a/MAINTAINERS b/MAINTAINERS
index 0bcba0d4994c..701d7115af74 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13817,7 +13817,7 @@ M:	Xiaowei Song <songxiaowei@hisilicon.com>
 M:	Binghui Wang <wangbinghui@hisilicon.com>
 L:	linux-pci@vger.kernel.org
 S:	Maintained
-F:	Documentation/devicetree/bindings/pci/kirin-pcie.txt
+F:	Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
 F:	drivers/pci/controller/dwc/pcie-kirin.c
 
 PCIE DRIVER FOR HISILICON STB
-- 
2.29.2


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

* [PATCH v2 03/11] doc: bindings: add new parameters used by Kirin 970
  2021-02-03  7:01 [PATCH v2 00/11] Add support for Hikey 970 PCIe Mauro Carvalho Chehab
  2021-02-03  7:01 ` [PATCH v2 01/11] doc: bindings: PCI: designware-pcie.txt: convert it to YAML Mauro Carvalho Chehab
  2021-02-03  7:01 ` [PATCH v2 02/11] doc: bindings: kirin-pcie.txt: " Mauro Carvalho Chehab
@ 2021-02-03  7:01 ` Mauro Carvalho Chehab
  2021-02-03  7:01 ` [PATCH v2 04/11] PCI: dwc: pcie-kirin: add support for Kirin 970 PCIe controller Mauro Carvalho Chehab
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 24+ messages in thread
From: Mauro Carvalho Chehab @ 2021-02-03  7:01 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Binghui Wang, Bjorn Helgaas, Rob Herring,
	Xiaowei Song, devicetree, linux-kernel, linux-pci

There are a few extra optional bindings that are needed for Kirin 970
based PCIe designs to work. Add them.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 .../bindings/pci/hisilicon,kirin-pcie.yaml    | 60 ++++++++++++++++++-
 1 file changed, 57 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
index 46f9f3f25dbc..7a58883e07ec 100644
--- a/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
@@ -34,8 +34,18 @@ properties:
       - const: config       # PCIe configuration space registers
 
   reset-gpios:
-    description: The GPIO to generate PCIe PERST# assert and deassert signal.
-    maxItems: 1
+    description: The GPIOs to generate PCIe PERST# assert and deassert signal.
+    minItems: 1
+    maxItems: 4
+
+  clkreq-gpios:
+    description: CLKREQ signal GPIO pins to be enabled during PCI power on
+    minItems: 1
+    maxItems: 3
+
+  eye_param:
+    description: items to adjust the eye parameters
+    maxItems: 5
 
 required:
   - compatible
@@ -52,12 +62,13 @@ examples:
   - |
     #include <dt-bindings/interrupt-controller/arm-gic.h>
     #include <dt-bindings/clock/hi3660-clock.h>
+    #include <dt-bindings/clock/hi3670-clock.h>
 
     soc {
       #address-cells = <2>;
       #size-cells = <2>;
 
-      pcie: pcie@f4000000 {
+      pcie1: pcie@f4000000 {
         compatible = "hisilicon,kirin960-pcie";
         reg = <0x0 0xf4000000 0x0 0x1000>,
               <0x0 0xff3fe000 0x0 0x1000>,
@@ -87,4 +98,47 @@ examples:
                       "pcie_apb_sys", "pcie_aclk";
         reset-gpios = <&gpio11 1 0 >;
       };
+
+      pcie2: pcie@f5000000 {
+        compatible = "hisilicon,kirin970-pcie";
+        reg = <0x0 0xf4000000 0x0 0x1000000>,
+              <0x0 0xfc180000 0x0 0x1000>,
+              <0x0 0xfc000000 0x0 0x80000>,
+              <0x0 0xf5000000 0x0 0x2000>;
+        pci-supply = <&ldo33>;
+        reg-names = "dbi", "apb", "phy", "config";
+        bus-range = <0x0  0x1>;
+        #address-cells = <3>;
+        #size-cells = <2>;
+        device_type = "pci";
+        ranges = <0x02000000 0x0 0x00000000 0x0 0xf6000000 0x0 0x02000000>;
+        num-lanes = <1>;
+        #interrupt-cells = <1>;
+        interrupts = <0 283 4>;
+        interrupt-names = "msi";
+        interrupt-map-mask = <0 0 0 7>;
+        interrupt-map = <0x0 0 0 1 &gic GIC_SPI 282 IRQ_TYPE_LEVEL_HIGH>,
+                        <0x0 0 0 2 &gic GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>,
+                        <0x0 0 0 3 &gic GIC_SPI 284 IRQ_TYPE_LEVEL_HIGH>,
+                        <0x0 0 0 4 &gic GIC_SPI 285 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&crg_ctrl HI3670_CLK_GATE_PCIEPHY_REF>,
+                 <&crg_ctrl HI3670_CLK_GATE_PCIEAUX>,
+                 <&crg_ctrl HI3670_PCLK_GATE_PCIE_PHY>,
+                 <&crg_ctrl HI3670_PCLK_GATE_PCIE_SYS>,
+                 <&crg_ctrl HI3670_ACLK_GATE_PCIE>;
+
+        clock-names = "pcie_phy_ref", "pcie_aux",
+                      "pcie_apb_phy", "pcie_apb_sys",
+                      "pcie_aclk";
+        reset-gpios = <&gpio7 0 0 >, <&gpio25 2 0 >,
+                      <&gpio3 1 0 >, <&gpio27 4 0 >;
+
+        clkreq-gpios = <&gpio20 6 0 >, <&gpio27 3 0 >, <&gpio17 0 0 >;
+
+        /* vboost iboost pre post main */
+        eye_param = <0xFFFFFFFF 0xFFFFFFFF 0xFFFFFFFF 0xFFFFFFFF 0xFFFFFFFF>;
+        msi-parent = <&its_pcie>;
+        pinctrl-names = "default";
+        pinctrl-0 = <&pcie_clkreq_pmx_func &pcie_clkreq_cfg_func>;
+      };
     };
-- 
2.29.2


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

* [PATCH v2 04/11] PCI: dwc: pcie-kirin: add support for Kirin 970 PCIe controller
  2021-02-03  7:01 [PATCH v2 00/11] Add support for Hikey 970 PCIe Mauro Carvalho Chehab
                   ` (2 preceding siblings ...)
  2021-02-03  7:01 ` [PATCH v2 03/11] doc: bindings: add new parameters used by Kirin 970 Mauro Carvalho Chehab
@ 2021-02-03  7:01 ` Mauro Carvalho Chehab
  2021-02-03  9:32   ` kernel test robot
  2021-02-03 14:34   ` Rob Herring
  2021-02-03  7:01 ` [PATCH v2 05/11] PCI: dwc: pcie-kirin: simplify error handling logic Mauro Carvalho Chehab
                   ` (6 subsequent siblings)
  10 siblings, 2 replies; 24+ messages in thread
From: Mauro Carvalho Chehab @ 2021-02-03  7:01 UTC (permalink / raw)
  Cc: Manivannan Sadhasivam, Binghui Wang, Bjorn Helgaas,
	Lorenzo Pieralisi, Rob Herring, Xiaowei Song, linux-kernel,
	linux-pci, Mauro Carvalho Chehab

From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Add support for HiSilicon Kirin 970 (hi3670) SoC PCIe controller, based
on Synopsys DesignWare PCIe controller IP.

[mchehab+huawei@kernel.org: fix merge conflicts]
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/pci/controller/dwc/pcie-kirin.c | 723 +++++++++++++++++++++++-
 1 file changed, 707 insertions(+), 16 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index 026fd1e42a55..5925d2b345a8 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -29,6 +29,7 @@
 #define to_kirin_pcie(x) dev_get_drvdata((x)->dev)
 
 #define REF_CLK_FREQ			100000000
+#define AXI_CLK_FREQ			207500000
 
 /* PCIe ELBI registers */
 #define SOC_PCIECTRL_CTRL0_ADDR		0x000
@@ -60,6 +61,65 @@
 #define PCIE_DEBOUNCE_PARAM	0xF0F400
 #define PCIE_OE_BYPASS		(0x3 << 28)
 
+/* PCIe CTRL registers */
+#define SOC_PCIECTRL_CTRL0_ADDR   0x000
+#define SOC_PCIECTRL_CTRL1_ADDR   0x004
+#define SOC_PCIECTRL_CTRL7_ADDR   0x01c
+#define SOC_PCIECTRL_CTRL12_ADDR  0x030
+#define SOC_PCIECTRL_CTRL20_ADDR  0x050
+#define SOC_PCIECTRL_CTRL21_ADDR  0x054
+#define SOC_PCIECTRL_STATE0_ADDR  0x400
+
+/* PCIe PHY registers */
+#define SOC_PCIEPHY_CTRL0_ADDR    0x000
+#define SOC_PCIEPHY_CTRL1_ADDR    0x004
+#define SOC_PCIEPHY_CTRL2_ADDR    0x008
+#define SOC_PCIEPHY_CTRL3_ADDR    0x00c
+#define SOC_PCIEPHY_CTRL38_ADDR   0x0098
+#define SOC_PCIEPHY_STATE0_ADDR   0x400
+
+#define PCIE_LINKUP_ENABLE            (0x8020)
+#define PCIE_ELBI_SLV_DBI_ENABLE      (0x1 << 21)
+#define PCIE_LTSSM_ENABLE_BIT         (0x1 << 11)
+#define PCIEPHY_RESET_BIT             (0x1 << 17)
+#define PCIEPHY_PIPE_LINE0_RESET_BIT  (0x1 << 19)
+
+#define PORT_MSI_CTRL_ADDR            0x820
+#define PORT_MSI_CTRL_UPPER_ADDR      0x824
+#define PORT_MSI_CTRL_INT0_ENABLE     0x828
+
+#define EYEPARAM_NOCFG 0xFFFFFFFF
+#define RAWLANEN_DIG_PCS_XF_TX_OVRD_IN_1 0x3001
+#define SUP_DIG_LVL_OVRD_IN 0xf
+#define LANEN_DIG_ASIC_TX_OVRD_IN_1 0x1002
+#define LANEN_DIG_ASIC_TX_OVRD_IN_2 0x1003
+
+/* kirin970 pciephy register */
+#define SOC_PCIEPHY_MMC1PLL_CTRL1  0xc04
+#define SOC_PCIEPHY_MMC1PLL_CTRL16 0xC40
+#define SOC_PCIEPHY_MMC1PLL_CTRL17 0xC44
+#define SOC_PCIEPHY_MMC1PLL_CTRL20 0xC50
+#define SOC_PCIEPHY_MMC1PLL_CTRL21 0xC54
+#define SOC_PCIEPHY_MMC1PLL_STAT0  0xE00
+
+#define CRGPERIPH_PEREN12   0x470
+#define CRGPERIPH_PERDIS12  0x474
+#define CRGPERIPH_PCIECTRL0 0x800
+
+/* define ie,oe cfg */
+#define IO_IE_EN_HARD_BYPASS         (0x1 << 27)
+#define IO_OE_EN_HARD_BYPASS         (0x1 << 11)
+#define IO_HARD_CTRL_DEBOUNCE_BYPASS (0x1 << 10)
+#define IO_OE_GT_MODE                (0x2 << 7)
+#define DEBOUNCE_WAITCFG_IN          (0xf << 20)
+#define DEBOUNCE_WAITCFG_OUT         (0xf << 13)
+
+/* noc power domain */
+#define NOC_POWER_IDLEREQ_1 0x38c
+#define NOC_POWER_IDLE_1    0x394
+#define NOC_PW_MASK         0x10000
+#define NOC_PW_SET_BIT      0x1
+
 /* peri_crg ctrl */
 #define CRGCTRL_PCIE_ASSERT_OFFSET	0x88
 #define CRGCTRL_PCIE_ASSERT_BIT		0x8c000000
@@ -84,12 +144,21 @@ struct kirin_pcie {
 	void __iomem	*phy_base;
 	struct regmap	*crgctrl;
 	struct regmap	*sysctrl;
+	struct regmap	*pmctrl;
 	struct clk	*apb_sys_clk;
 	struct clk	*apb_phy_clk;
 	struct clk	*phy_ref_clk;
 	struct clk	*pcie_aclk;
 	struct clk	*pcie_aux_clk;
-	int		gpio_id_reset;
+	int		gpio_id_reset[4];
+	int		gpio_id_clkreq[3];
+	u32		eye_param[5];
+};
+
+struct kirin_pcie_ops {
+	long (*get_resource)(struct kirin_pcie *kirin_pcie,
+			     struct platform_device *pdev);
+	int (*power_on)(struct kirin_pcie *kirin_pcie);
 };
 
 /* Registers in PCIeCTRL */
@@ -116,6 +185,28 @@ static inline u32 kirin_apb_phy_readl(struct kirin_pcie *kirin_pcie, u32 reg)
 	return readl(kirin_pcie->phy_base + reg);
 }
 
+static inline void kirin970_apb_phy_writel(struct kirin_pcie *kirin_pcie,
+					u32 val, u32 reg)
+{
+	writel(val, kirin_pcie->phy_base + 0x40000 + reg);
+}
+
+static inline u32 kirin970_apb_phy_readl(struct kirin_pcie *kirin_pcie, u32 reg)
+{
+	return readl(kirin_pcie->phy_base + 0x40000 + reg);
+}
+
+static inline void kirin_apb_natural_phy_writel(struct kirin_pcie *kirin_pcie,
+					u32 val, u32 reg)
+{
+	writel(val, kirin_pcie->phy_base + reg * 4);
+}
+
+static inline u32 kirin_apb_natural_phy_readl(struct kirin_pcie *kirin_pcie, u32 reg)
+{
+	return readl(kirin_pcie->phy_base + reg * 4);
+}
+
 static long kirin_pcie_get_clk(struct kirin_pcie *kirin_pcie,
 			       struct platform_device *pdev)
 {
@@ -144,9 +235,68 @@ static long kirin_pcie_get_clk(struct kirin_pcie *kirin_pcie,
 	return 0;
 }
 
-static long kirin_pcie_get_resource(struct kirin_pcie *kirin_pcie,
-				    struct platform_device *pdev)
+void kirin970_pcie_get_eyeparam(struct kirin_pcie *pcie)
 {
+	struct device *dev = pcie->pci->dev;
+	int i;
+	struct device_node *np;
+
+	np = dev->of_node;
+
+	if (of_property_read_u32_array(np, "eye_param", pcie->eye_param, 5)) {
+		for (i = 0; i < 5; i++)
+		pcie->eye_param[i] = EYEPARAM_NOCFG;
+	}
+
+	dev_dbg(dev, "eye_param_vboost = [0x%x]\n", pcie->eye_param[0]);
+	dev_dbg(dev, "eye_param_iboost = [0x%x]\n", pcie->eye_param[1]);
+	dev_dbg(dev, "eye_param_pre = [0x%x]\n", pcie->eye_param[2]);
+	dev_dbg(dev, "eye_param_post = [0x%x]\n", pcie->eye_param[3]);
+	dev_dbg(dev, "eye_param_main = [0x%x]\n", pcie->eye_param[4]);
+}
+
+static void kirin970_pcie_set_eyeparam(struct kirin_pcie *kirin_pcie)
+{
+	u32 val;
+
+	val = kirin_apb_natural_phy_readl(kirin_pcie, RAWLANEN_DIG_PCS_XF_TX_OVRD_IN_1);
+
+	if (kirin_pcie->eye_param[1] != EYEPARAM_NOCFG) {
+		val &= (~0xf00);
+		val |= (kirin_pcie->eye_param[1] << 8) | (0x1 << 12);
+	}
+	kirin_apb_natural_phy_writel(kirin_pcie, val, RAWLANEN_DIG_PCS_XF_TX_OVRD_IN_1);
+
+	val = kirin_apb_natural_phy_readl(kirin_pcie, LANEN_DIG_ASIC_TX_OVRD_IN_2);
+	val &= (~0x1FBF);
+	if (kirin_pcie->eye_param[2] != EYEPARAM_NOCFG)
+		val |= (kirin_pcie->eye_param[2]<< 0) | (0x1 << 6);
+
+	if (kirin_pcie->eye_param[3] != EYEPARAM_NOCFG)
+		val |= (kirin_pcie->eye_param[3] << 7) | (0x1 << 13);
+
+	kirin_apb_natural_phy_writel(kirin_pcie, val, LANEN_DIG_ASIC_TX_OVRD_IN_2);
+
+	val = kirin_apb_natural_phy_readl(kirin_pcie, SUP_DIG_LVL_OVRD_IN);
+	if (kirin_pcie->eye_param[0] != EYEPARAM_NOCFG) {
+		val &= (~0x1C0);
+		val |= (kirin_pcie->eye_param[0] << 6) | (0x1 << 9);
+	}
+	kirin_apb_natural_phy_writel(kirin_pcie, val, SUP_DIG_LVL_OVRD_IN);
+
+	val = kirin_apb_natural_phy_readl(kirin_pcie, LANEN_DIG_ASIC_TX_OVRD_IN_1);
+	if (kirin_pcie->eye_param[4] != EYEPARAM_NOCFG) {
+		val &= (~0x7E00);
+		val |= (kirin_pcie->eye_param[4] << 9) | (0x1 << 15);
+	}
+	kirin_apb_natural_phy_writel(kirin_pcie, val, LANEN_DIG_ASIC_TX_OVRD_IN_1);
+}
+
+static long kirin960_pcie_get_resource(struct kirin_pcie *kirin_pcie,
+				       struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+
 	kirin_pcie->apb_base =
 		devm_platform_ioremap_resource_byname(pdev, "apb");
 	if (IS_ERR(kirin_pcie->apb_base))
@@ -167,6 +317,122 @@ static long kirin_pcie_get_resource(struct kirin_pcie *kirin_pcie,
 	if (IS_ERR(kirin_pcie->sysctrl))
 		return PTR_ERR(kirin_pcie->sysctrl);
 
+	kirin_pcie->gpio_id_reset[0] = of_get_named_gpio(dev->of_node,
+						      "reset-gpios", 0);
+	if (kirin_pcie->gpio_id_reset[0] < 0)
+		return -ENODEV;
+
+	return 0;
+}
+
+static long kirin970_pcie_get_resource(struct kirin_pcie *kirin_pcie,
+				      struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct resource *apb;
+	struct resource *phy;
+	struct resource *dbi;
+	int ret;
+
+	apb = platform_get_resource_byname(pdev, IORESOURCE_MEM, "apb");
+	kirin_pcie->apb_base = devm_ioremap_resource(dev, apb);
+	if (IS_ERR(kirin_pcie->apb_base))
+		return PTR_ERR(kirin_pcie->apb_base);
+
+	phy = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy");
+	kirin_pcie->phy_base = devm_ioremap_resource(dev, phy);
+	if (IS_ERR(kirin_pcie->phy_base))
+		return PTR_ERR(kirin_pcie->phy_base);
+
+	dbi = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
+	kirin_pcie->pci->dbi_base = devm_ioremap_resource(dev, dbi);
+	if (IS_ERR(kirin_pcie->pci->dbi_base))
+		return PTR_ERR(kirin_pcie->pci->dbi_base);
+
+	kirin970_pcie_get_eyeparam(kirin_pcie);
+
+	kirin_pcie->gpio_id_reset[0] = of_get_named_gpio(dev->of_node,
+						"switch,reset-gpios", 0);
+	if (kirin_pcie->gpio_id_reset[0] < 0)
+		return -ENODEV;
+
+	kirin_pcie->gpio_id_reset[1] = of_get_named_gpio(dev->of_node,
+						"eth,reset-gpios", 0);
+	if (kirin_pcie->gpio_id_reset[1] < 0)
+		return -ENODEV;
+
+	kirin_pcie->gpio_id_reset[2] = of_get_named_gpio(dev->of_node,
+						"m_2,reset-gpios", 0);
+	if (kirin_pcie->gpio_id_reset[2] < 0)
+		return -ENODEV;
+
+	kirin_pcie->gpio_id_reset[3] = of_get_named_gpio(dev->of_node,
+						"mini1,reset-gpios", 0);
+	if (kirin_pcie->gpio_id_reset[3] < 0)
+		return -ENODEV;
+
+	ret = devm_gpio_request(dev, kirin_pcie->gpio_id_reset[0],
+				    "pcie_switch_reset");
+	if (ret)
+		return ret;
+	ret = devm_gpio_request(dev, kirin_pcie->gpio_id_reset[1],
+				    "pcie_eth_reset");
+	if (ret)
+		return ret;
+	ret = devm_gpio_request(dev, kirin_pcie->gpio_id_reset[2],
+				    "pcie_m_2_reset");
+	if (ret)
+		return ret;
+	ret = devm_gpio_request(dev, kirin_pcie->gpio_id_reset[3],
+				    "pcie_mini1_reset");
+	if (ret)
+		return ret;
+
+	kirin_pcie->gpio_id_clkreq[0] = of_get_named_gpio(dev->of_node,
+						"eth,clkreq-gpios", 0);
+	if (kirin_pcie->gpio_id_clkreq[0] < 0)
+		return -ENODEV;
+
+	kirin_pcie->gpio_id_clkreq[1] = of_get_named_gpio(dev->of_node,
+						"m_2,clkreq-gpios", 0);
+	if (kirin_pcie->gpio_id_clkreq[1] < 0)
+		return -ENODEV;
+
+	kirin_pcie->gpio_id_clkreq[2] = of_get_named_gpio(dev->of_node,
+						"mini1,clkreq-gpios", 0);
+	if (kirin_pcie->gpio_id_clkreq[2] < 0)
+		return -ENODEV;
+
+	ret = devm_gpio_request(dev, kirin_pcie->gpio_id_clkreq[0],
+				    "pcie_eth_clkreq");
+	if (ret)
+		return ret;
+
+	ret = devm_gpio_request(dev, kirin_pcie->gpio_id_clkreq[1],
+				    "pcie_m_2_clkreq");
+	if (ret)
+		return ret;
+
+	ret = devm_gpio_request(dev, kirin_pcie->gpio_id_clkreq[2],
+				    "pcie_mini1_clkreq");
+	if (ret)
+		return ret;
+
+	kirin_pcie->crgctrl =
+		syscon_regmap_lookup_by_compatible("hisilicon,hi3670-crgctrl");
+	if (IS_ERR(kirin_pcie->crgctrl))
+		return PTR_ERR(kirin_pcie->crgctrl);
+
+	kirin_pcie->sysctrl =
+		syscon_regmap_lookup_by_compatible("hisilicon,hi3670-sctrl");
+	if (IS_ERR(kirin_pcie->sysctrl))
+		return PTR_ERR(kirin_pcie->sysctrl);
+
+	kirin_pcie->pmctrl =
+		syscon_regmap_lookup_by_compatible("hisilicon,hi3670-pmctrl");
+	if (IS_ERR(kirin_pcie->sysctrl))
+		return PTR_ERR(kirin_pcie->sysctrl);
+
 	return 0;
 }
 
@@ -208,6 +474,21 @@ static void kirin_pcie_oe_enable(struct kirin_pcie *kirin_pcie)
 	regmap_write(kirin_pcie->sysctrl, SCTRL_PCIE_OE_OFFSET, val);
 }
 
+static int kirin970_pcie_clk_ctrl(struct clk *clk, int clk_on)
+{
+	int ret = 0;
+
+	if (clk_on) {
+		ret = clk_prepare_enable(clk);
+		if (ret)
+			return ret;
+	} else {
+		clk_disable_unprepare(clk);
+	}
+
+	return ret;
+}
+
 static int kirin_pcie_clk_ctrl(struct kirin_pcie *kirin_pcie, bool enable)
 {
 	int ret = 0;
@@ -255,7 +536,401 @@ static int kirin_pcie_clk_ctrl(struct kirin_pcie *kirin_pcie, bool enable)
 	return ret;
 }
 
-static int kirin_pcie_power_on(struct kirin_pcie *kirin_pcie)
+static void kirin970_pcie_natural_cfg(struct kirin_pcie *kirin_pcie)
+{
+	u32 val;
+
+	/* change 2p mem_ctrl */
+	kirin_apb_ctrl_writel(kirin_pcie, 0x02605550, SOC_PCIECTRL_CTRL20_ADDR);
+
+	/* pull up sys_aux_pwr_det */
+	val = kirin_apb_ctrl_readl(kirin_pcie, SOC_PCIECTRL_CTRL7_ADDR);
+	val |= (0x1 << 10);
+	kirin_apb_ctrl_writel(kirin_pcie, val, SOC_PCIECTRL_CTRL7_ADDR);
+
+	/* output, pull down */
+	val = kirin_apb_ctrl_readl(kirin_pcie, SOC_PCIECTRL_CTRL12_ADDR);
+	val &= ~(0x3 << 2);
+	val |= (0x1 << 1);
+	val &= ~(0x1 << 0);
+	kirin_apb_ctrl_writel(kirin_pcie, val, SOC_PCIECTRL_CTRL12_ADDR);
+
+	/* Handle phy_reset and lane0_reset to HW */
+	val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_CTRL1_ADDR);
+	val |= PCIEPHY_RESET_BIT;
+	val &= ~PCIEPHY_PIPE_LINE0_RESET_BIT;
+	kirin970_apb_phy_writel(kirin_pcie, val, SOC_PCIEPHY_CTRL1_ADDR);
+
+	/* fix chip bug: TxDetectRx fail */
+	val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_CTRL38_ADDR);
+	val |= (0x1 << 2);
+	kirin970_apb_phy_writel(kirin_pcie, val, SOC_PCIEPHY_CTRL38_ADDR);
+}
+
+static void kirin970_pcie_pll_init(struct kirin_pcie *kirin_pcie)
+{
+	u32 val;
+
+	/* choose FNPLL */
+	val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_MMC1PLL_CTRL1);
+	val |= (0x1 << 27);
+	kirin970_apb_phy_writel(kirin_pcie, val, SOC_PCIEPHY_MMC1PLL_CTRL1);
+
+	val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_MMC1PLL_CTRL16);
+	val &= 0xF000FFFF;
+	/* fnpll fbdiv = 0xD0 */
+	val |= (0xd0 << 16);
+	kirin970_apb_phy_writel(kirin_pcie, val, SOC_PCIEPHY_MMC1PLL_CTRL16);
+
+	val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_MMC1PLL_CTRL17);
+	val &= 0xFF000000;
+	/* fnpll fracdiv = 0x555555 */
+	val |= (0x555555 << 0);
+	kirin970_apb_phy_writel(kirin_pcie, val, SOC_PCIEPHY_MMC1PLL_CTRL17);
+
+	val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_MMC1PLL_CTRL20);
+	val &= 0xF5FF88FF;
+	/* fnpll dll_en = 0x1 */
+	val |= (0x1 << 27);
+	/* fnpll postdiv1 = 0x5 */
+	val |= (0x5 << 8);
+	/* fnpll postdiv2 = 0x4 */
+	val |= (0x4 << 12);
+	/* fnpll pll_mode = 0x0 */
+	val &= ~(0x1 << 25);
+	kirin970_apb_phy_writel(kirin_pcie, val, SOC_PCIEPHY_MMC1PLL_CTRL20);
+
+	kirin970_apb_phy_writel(kirin_pcie, 0x20, SOC_PCIEPHY_MMC1PLL_CTRL21);
+}
+
+static int kirin970_pcie_pll_ctrl(struct kirin_pcie *kirin_pcie, bool enable)
+{
+	struct device *dev = kirin_pcie->pci->dev;
+	u32 val;
+	int time = 200;
+
+	if (enable) {
+		/* pd = 0 */
+		val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_MMC1PLL_CTRL16);
+		val &= ~(0x1 << 0);
+		kirin970_apb_phy_writel(kirin_pcie, val, SOC_PCIEPHY_MMC1PLL_CTRL16);
+
+		val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_MMC1PLL_STAT0);
+
+		/* choose FNPLL */
+		while (!(val & 0x10)) {
+			if (!time) {
+				dev_err(dev, "wait for pll_lock timeout\n");
+				return -1;
+			}
+			time --;
+			udelay(1);
+			val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_MMC1PLL_STAT0);
+		}
+
+		/* pciepll_bp = 0 */
+		val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_MMC1PLL_CTRL20);
+		val &= ~(0x1 << 16);
+		kirin970_apb_phy_writel(kirin_pcie, val, SOC_PCIEPHY_MMC1PLL_CTRL20);
+
+	} else {
+		/* pd = 1 */
+		val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_MMC1PLL_CTRL16);
+		val |= (0x1 << 0);
+		kirin970_apb_phy_writel(kirin_pcie, val, SOC_PCIEPHY_MMC1PLL_CTRL16);
+
+		/* pciepll_bp = 1 */
+		val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_MMC1PLL_CTRL20);
+		val |= (0x1 << 16);
+		kirin970_apb_phy_writel(kirin_pcie, val, SOC_PCIEPHY_MMC1PLL_CTRL20);
+	}
+
+	 return 0;
+}
+
+static void kirin970_pcie_hp_debounce_gt(struct kirin_pcie *kirin_pcie, bool open)
+{
+	if (open)
+		/* gt_clk_pcie_hp/gt_clk_pcie_debounce open */
+		regmap_write(kirin_pcie->crgctrl, CRGPERIPH_PEREN12, 0x9000);
+	else
+		/* gt_clk_pcie_hp/gt_clk_pcie_debounce close */
+		regmap_write(kirin_pcie->crgctrl, CRGPERIPH_PERDIS12, 0x9000);
+}
+
+static void kirin970_pcie_phyref_gt(struct kirin_pcie *kirin_pcie, bool open)
+{
+	unsigned int val;
+
+	regmap_read(kirin_pcie->crgctrl, CRGPERIPH_PCIECTRL0, &val);
+
+	if (open)
+		val &= ~(0x1 << 1); //enable hard gt mode
+	else
+		val |= (0x1 << 1); //disable hard gt mode
+
+	regmap_write(kirin_pcie->crgctrl, CRGPERIPH_PCIECTRL0, val);
+
+	/* disable soft gt mode */
+	regmap_write(kirin_pcie->crgctrl, CRGPERIPH_PERDIS12, 0x4000);
+}
+
+static void kirin970_pcie_oe_ctrl(struct kirin_pcie *kirin_pcie, bool en_flag)
+{
+	unsigned int val;
+
+	regmap_read(kirin_pcie->crgctrl , CRGPERIPH_PCIECTRL0, &val);
+
+	/* set ie cfg */
+	val |= IO_IE_EN_HARD_BYPASS;
+
+	/* set oe cfg */
+	val &= ~IO_HARD_CTRL_DEBOUNCE_BYPASS;
+
+	/* set phy_debounce in&out time */
+	val |= (DEBOUNCE_WAITCFG_IN | DEBOUNCE_WAITCFG_OUT);
+
+	/* select oe_gt_mode */
+	val |= IO_OE_GT_MODE;
+
+	if (en_flag)
+		val &= ~IO_OE_EN_HARD_BYPASS;
+	else
+		val |= IO_OE_EN_HARD_BYPASS;
+
+	regmap_write(kirin_pcie->crgctrl, CRGPERIPH_PCIECTRL0, val);
+}
+
+static void kirin970_pcie_ioref_gt(struct kirin_pcie *kirin_pcie, bool open)
+{
+	unsigned int val;
+
+	if (open) {
+		kirin_apb_ctrl_writel(kirin_pcie, 0x20000070, SOC_PCIECTRL_CTRL21_ADDR);
+
+		kirin970_pcie_oe_ctrl(kirin_pcie, true);
+
+		/* en hard gt mode */
+		regmap_read(kirin_pcie->crgctrl, CRGPERIPH_PCIECTRL0, &val);
+		val &= ~(0x1 << 0);
+		regmap_write(kirin_pcie->crgctrl, CRGPERIPH_PCIECTRL0, val);
+
+		/* disable soft gt mode */
+		regmap_write(kirin_pcie->crgctrl, CRGPERIPH_PERDIS12, 0x2000);
+
+	} else {
+		/* disable hard gt mode */
+		regmap_read(kirin_pcie->crgctrl, CRGPERIPH_PCIECTRL0, &val);
+		val |= (0x1 << 0);
+		regmap_write(kirin_pcie->crgctrl, CRGPERIPH_PCIECTRL0, val);
+
+		/* disable soft gt mode */
+		regmap_write(kirin_pcie->crgctrl, CRGPERIPH_PERDIS12, 0x2000);
+
+		kirin970_pcie_oe_ctrl(kirin_pcie, false);
+       }
+}
+
+static int kirin970_pcie_allclk_ctrl(struct kirin_pcie *kirin_pcie, bool clk_on)
+{
+	struct device *dev = kirin_pcie->pci->dev;
+	u32 val;
+	int ret = 0;
+
+	if (!clk_on)
+		goto ALL_CLOSE;
+
+	/* choose 100MHz clk src: Bit[8]==1 pad, Bit[8]==0 pll */
+	val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_CTRL1_ADDR);
+	val &= ~(0x1 << 8);
+	kirin970_apb_phy_writel(kirin_pcie, val, SOC_PCIEPHY_CTRL1_ADDR);
+
+	kirin970_pcie_pll_init(kirin_pcie);
+
+	ret = kirin970_pcie_pll_ctrl(kirin_pcie, true);
+	if (ret) {
+		dev_err(dev, "Failed to enable pll\n");
+		return -1;
+	}
+	kirin970_pcie_hp_debounce_gt(kirin_pcie, true);
+	kirin970_pcie_phyref_gt(kirin_pcie, true);
+	kirin970_pcie_ioref_gt(kirin_pcie, true);
+
+	ret = clk_set_rate(kirin_pcie->pcie_aclk, AXI_CLK_FREQ);
+	if (ret) {
+		dev_err(dev, "Failed to set rate\n");
+		goto GT_CLOSE;
+	}
+
+	ret = kirin970_pcie_clk_ctrl(kirin_pcie->pcie_aclk, true);
+	if (ret) {
+		dev_err(dev, "Failed to enable pcie_aclk\n");
+		goto GT_CLOSE;
+	}
+
+	ret = kirin970_pcie_clk_ctrl(kirin_pcie->pcie_aux_clk, true);
+	if (ret) {
+		dev_err(dev, "Failed to enable pcie_aux_clk\n");
+		goto AUX_CLK_FAIL;
+	}
+
+	return 0;
+
+ALL_CLOSE:
+	kirin970_pcie_clk_ctrl(kirin_pcie->pcie_aux_clk, false);
+AUX_CLK_FAIL:
+	kirin970_pcie_clk_ctrl(kirin_pcie->pcie_aclk, false);
+GT_CLOSE:
+	kirin970_pcie_ioref_gt(kirin_pcie, false);
+	kirin970_pcie_phyref_gt(kirin_pcie, false);
+	kirin970_pcie_hp_debounce_gt(kirin_pcie, false);
+
+	kirin970_pcie_pll_ctrl(kirin_pcie, false);
+
+	return ret;
+}
+
+static bool is_pipe_clk_stable(struct kirin_pcie *kirin_pcie)
+{
+	struct device *dev = kirin_pcie->pci->dev;
+	u32 val;
+	u32 time = 100;
+	u32 pipe_clk_stable = 0x1 << 19;
+
+	val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_STATE0_ADDR);
+	while (val & pipe_clk_stable) {
+		mdelay(1);
+		if (time == 0) {
+			dev_err(dev, "PIPE clk is not stable\n");
+			return false;
+		}
+		time--;
+		val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_STATE0_ADDR);
+	}
+
+	return true;
+}
+
+static int kirin970_pcie_noc_power(struct kirin_pcie *kirin_pcie, bool enable)
+{
+	struct device *dev = kirin_pcie->pci->dev;
+	u32 time = 100;
+	unsigned int val = NOC_PW_MASK;
+	int rst;
+
+	if (enable)
+		val = NOC_PW_MASK | NOC_PW_SET_BIT;
+	else
+		val = NOC_PW_MASK;
+	rst = enable ? 1 : 0;
+
+	regmap_write(kirin_pcie->pmctrl, NOC_POWER_IDLEREQ_1, val);
+
+	time = 100;
+	regmap_read(kirin_pcie->pmctrl, NOC_POWER_IDLE_1, &val);
+	while((val & NOC_PW_SET_BIT) != rst) {
+		udelay(10);
+		if (!time) {
+			dev_err(dev, "Failed to reverse noc power-status\n");
+			return -1;
+		}
+		time--;
+		regmap_read(kirin_pcie->pmctrl, NOC_POWER_IDLE_1, &val);
+	}
+
+	return 0;
+}
+
+static int kirin970_pcie_power_on(struct kirin_pcie *kirin_pcie)
+{
+	struct device *dev = kirin_pcie->pci->dev;
+	int ret;
+	u32 val;
+
+	/* Power supply for Host */
+	regmap_write(kirin_pcie->sysctrl,
+		     SCTRL_PCIE_CMOS_OFFSET, SCTRL_PCIE_CMOS_BIT);
+	usleep_range(TIME_CMOS_MIN, TIME_CMOS_MAX);
+	kirin_pcie_oe_enable(kirin_pcie);
+
+	ret = gpio_direction_output(kirin_pcie->gpio_id_clkreq[0], 0);
+	if (ret)
+		dev_err(dev, "Failed to pulse eth clkreq signal\n");
+
+	ret = gpio_direction_output(kirin_pcie->gpio_id_clkreq[1], 0);
+	if (ret)
+		dev_err(dev, "Failed to pulse m.2 clkreq signal\n");
+
+	ret = gpio_direction_output(kirin_pcie->gpio_id_clkreq[2], 0);
+	if (ret)
+		dev_err(dev, "Failed to pulse mini1 clkreq signal\n");
+
+	ret = kirin_pcie_clk_ctrl(kirin_pcie, true);
+	if (ret)
+		return ret;
+
+	/* ISO disable, PCIeCtrl, PHY assert and clk gate clear */
+	regmap_write(kirin_pcie->sysctrl,
+		     SCTRL_PCIE_ISO_OFFSET, SCTRL_PCIE_ISO_BIT);
+	regmap_write(kirin_pcie->crgctrl,
+		     CRGCTRL_PCIE_ASSERT_OFFSET, CRGCTRL_PCIE_ASSERT_BIT);
+	regmap_write(kirin_pcie->sysctrl,
+		     SCTRL_PCIE_HPCLK_OFFSET, SCTRL_PCIE_HPCLK_BIT);
+
+	kirin970_pcie_natural_cfg(kirin_pcie);
+
+	ret = kirin970_pcie_allclk_ctrl(kirin_pcie, true);
+	if (ret)
+		goto close_clk;
+
+	/* pull down phy_test_powerdown signal */
+	val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_CTRL0_ADDR);
+	val &= ~(0x1 << 22);
+	kirin970_apb_phy_writel(kirin_pcie, val, SOC_PCIEPHY_CTRL0_ADDR);
+
+	/* deassert controller perst_n */
+	val = kirin_apb_ctrl_readl(kirin_pcie, SOC_PCIECTRL_CTRL12_ADDR);
+	val |= (0x1 << 2);
+	kirin_apb_ctrl_writel(kirin_pcie, val, SOC_PCIECTRL_CTRL12_ADDR);
+	udelay(10);
+
+	/* perst assert Endpoints */
+	usleep_range(21000, 23000);
+	ret = gpio_direction_output(kirin_pcie->gpio_id_reset[0], 1);
+	if (ret)
+		goto close_clk;
+
+	ret = gpio_direction_output(kirin_pcie->gpio_id_reset[1], 1);
+	if (ret)
+		goto close_clk;
+
+	ret = gpio_direction_output(kirin_pcie->gpio_id_reset[2], 1);
+	if (ret)
+		goto close_clk;
+
+	ret = gpio_direction_output(kirin_pcie->gpio_id_reset[3], 1);
+	if (ret)
+		goto close_clk;
+
+	usleep_range(10000, 11000);
+
+	ret = is_pipe_clk_stable(kirin_pcie);
+	if (!ret)
+		goto close_clk;
+
+	kirin970_pcie_set_eyeparam(kirin_pcie);
+
+	ret = kirin970_pcie_noc_power(kirin_pcie, false);
+	if (ret)
+		goto close_clk;
+
+	return 0;
+close_clk:
+	kirin_pcie_clk_ctrl(kirin_pcie, false);
+	return ret;
+}
+
+static int kirin960_pcie_power_on(struct kirin_pcie *kirin_pcie)
 {
 	int ret;
 
@@ -282,9 +957,9 @@ static int kirin_pcie_power_on(struct kirin_pcie *kirin_pcie)
 		goto close_clk;
 
 	/* perst assert Endpoint */
-	if (!gpio_request(kirin_pcie->gpio_id_reset, "pcie_perst")) {
+	if (!gpio_request(kirin_pcie->gpio_id_reset[0], "pcie_perst")) {
 		usleep_range(REF_2_PERST_MIN, REF_2_PERST_MAX);
-		ret = gpio_direction_output(kirin_pcie->gpio_id_reset, 1);
+		ret = gpio_direction_output(kirin_pcie->gpio_id_reset[0], 1);
 		if (ret)
 			goto close_clk;
 		usleep_range(PERST_2_ACCESS_MIN, PERST_2_ACCESS_MAX);
@@ -419,11 +1094,29 @@ static const struct dw_pcie_host_ops kirin_pcie_host_ops = {
 	.host_init = kirin_pcie_host_init,
 };
 
+struct kirin_pcie_ops kirin960_pcie_ops = {
+	.get_resource = kirin960_pcie_get_resource,
+	.power_on = kirin960_pcie_power_on,
+};
+
+struct kirin_pcie_ops kirin970_pcie_ops = {
+	.get_resource = kirin970_pcie_get_resource,
+	.power_on = kirin970_pcie_power_on,
+};
+
+static const struct of_device_id kirin_pcie_match[] = {
+	{ .compatible = "hisilicon,kirin960-pcie", .data = &kirin960_pcie_ops },
+	{ .compatible = "hisilicon,kirin970-pcie", .data = &kirin970_pcie_ops },
+	{},
+};
+
 static int kirin_pcie_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct kirin_pcie *kirin_pcie;
 	struct dw_pcie *pci;
+	const struct of_device_id *of_id;
+	struct kirin_pcie_ops *ops;
 	int ret;
 
 	if (!dev->of_node) {
@@ -431,6 +1124,9 @@ static int kirin_pcie_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	of_id = of_match_node(kirin_pcie_match, dev->of_node);
+	ops = (struct kirin_pcie_ops *)of_id->data;
+
 	kirin_pcie = devm_kzalloc(dev, sizeof(struct kirin_pcie), GFP_KERNEL);
 	if (!kirin_pcie)
 		return -ENOMEM;
@@ -448,20 +1144,20 @@ static int kirin_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ret = kirin_pcie_get_resource(kirin_pcie, pdev);
+	ret = ops->get_resource(kirin_pcie, pdev);
 	if (ret)
 		return ret;
 
-	kirin_pcie->gpio_id_reset = of_get_named_gpio(dev->of_node,
+	kirin_pcie->gpio_id_reset[0] = of_get_named_gpio(dev->of_node,
 						      "reset-gpios", 0);
-	if (kirin_pcie->gpio_id_reset == -EPROBE_DEFER) {
+	if (kirin_pcie->gpio_id_reset[0] == -EPROBE_DEFER) {
 		return -EPROBE_DEFER;
-	} else if (!gpio_is_valid(kirin_pcie->gpio_id_reset)) {
+	} else if (!gpio_is_valid(kirin_pcie->gpio_id_reset[0])) {
 		dev_err(dev, "unable to get a valid gpio pin\n");
 		return -ENODEV;
 	}
 
-	ret = kirin_pcie_power_on(kirin_pcie);
+	ret = ops->power_on(kirin_pcie);
 	if (ret)
 		return ret;
 
@@ -470,11 +1166,6 @@ static int kirin_pcie_probe(struct platform_device *pdev)
 	return dw_pcie_host_init(&pci->pp);
 }
 
-static const struct of_device_id kirin_pcie_match[] = {
-	{ .compatible = "hisilicon,kirin960-pcie" },
-	{},
-};
-
 static struct platform_driver kirin_pcie_driver = {
 	.probe			= kirin_pcie_probe,
 	.driver			= {
-- 
2.29.2


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

* [PATCH v2 05/11] PCI: dwc: pcie-kirin: simplify error handling logic
  2021-02-03  7:01 [PATCH v2 00/11] Add support for Hikey 970 PCIe Mauro Carvalho Chehab
                   ` (3 preceding siblings ...)
  2021-02-03  7:01 ` [PATCH v2 04/11] PCI: dwc: pcie-kirin: add support for Kirin 970 PCIe controller Mauro Carvalho Chehab
@ 2021-02-03  7:01 ` Mauro Carvalho Chehab
  2021-02-03  7:01 ` [PATCH v2 06/11] PCI: dwc: pcie-kirin: simplify Kirin 970 get resource logic Mauro Carvalho Chehab
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 24+ messages in thread
From: Mauro Carvalho Chehab @ 2021-02-03  7:01 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Binghui Wang, Bjorn Helgaas,
	Lorenzo Pieralisi, Rob Herring, Xiaowei Song, linux-kernel,
	linux-pci

Instead of returning -ENODEV when of_get_named_gpio() fails, make it
return the actual error code.

With that, there's no need anymore to check for -EPROBE_DEFER at
kirin_pcie_probe().

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/pci/controller/dwc/pcie-kirin.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index 5925d2b345a8..f46a51ffd2c8 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -320,7 +320,7 @@ static long kirin960_pcie_get_resource(struct kirin_pcie *kirin_pcie,
 	kirin_pcie->gpio_id_reset[0] = of_get_named_gpio(dev->of_node,
 						      "reset-gpios", 0);
 	if (kirin_pcie->gpio_id_reset[0] < 0)
-		return -ENODEV;
+		return kirin_pcie->gpio_id_reset[0];
 
 	return 0;
 }
@@ -354,22 +354,22 @@ static long kirin970_pcie_get_resource(struct kirin_pcie *kirin_pcie,
 	kirin_pcie->gpio_id_reset[0] = of_get_named_gpio(dev->of_node,
 						"switch,reset-gpios", 0);
 	if (kirin_pcie->gpio_id_reset[0] < 0)
-		return -ENODEV;
+		return kirin_pcie->gpio_id_reset[0];
 
 	kirin_pcie->gpio_id_reset[1] = of_get_named_gpio(dev->of_node,
 						"eth,reset-gpios", 0);
 	if (kirin_pcie->gpio_id_reset[1] < 0)
-		return -ENODEV;
+		return kirin_pcie->gpio_id_reset[1];
 
 	kirin_pcie->gpio_id_reset[2] = of_get_named_gpio(dev->of_node,
 						"m_2,reset-gpios", 0);
 	if (kirin_pcie->gpio_id_reset[2] < 0)
-		return -ENODEV;
+		return kirin_pcie->gpio_id_reset[2];
 
 	kirin_pcie->gpio_id_reset[3] = of_get_named_gpio(dev->of_node,
 						"mini1,reset-gpios", 0);
 	if (kirin_pcie->gpio_id_reset[3] < 0)
-		return -ENODEV;
+		return kirin_pcie->gpio_id_reset[3];
 
 	ret = devm_gpio_request(dev, kirin_pcie->gpio_id_reset[0],
 				    "pcie_switch_reset");
@@ -1148,11 +1148,7 @@ static int kirin_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	kirin_pcie->gpio_id_reset[0] = of_get_named_gpio(dev->of_node,
-						      "reset-gpios", 0);
-	if (kirin_pcie->gpio_id_reset[0] == -EPROBE_DEFER) {
-		return -EPROBE_DEFER;
-	} else if (!gpio_is_valid(kirin_pcie->gpio_id_reset[0])) {
+	if (!gpio_is_valid(kirin_pcie->gpio_id_reset[0])) {
 		dev_err(dev, "unable to get a valid gpio pin\n");
 		return -ENODEV;
 	}
-- 
2.29.2


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

* [PATCH v2 06/11] PCI: dwc: pcie-kirin: simplify Kirin 970 get resource logic
  2021-02-03  7:01 [PATCH v2 00/11] Add support for Hikey 970 PCIe Mauro Carvalho Chehab
                   ` (4 preceding siblings ...)
  2021-02-03  7:01 ` [PATCH v2 05/11] PCI: dwc: pcie-kirin: simplify error handling logic Mauro Carvalho Chehab
@ 2021-02-03  7:01 ` Mauro Carvalho Chehab
  2021-02-03  7:01 ` [PATCH v2 07/11] PCI: dwc: pcie-kirin: place common init code altogether Mauro Carvalho Chehab
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 24+ messages in thread
From: Mauro Carvalho Chehab @ 2021-02-03  7:01 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Binghui Wang, Bjorn Helgaas,
	Lorenzo Pieralisi, Rob Herring, Xiaowei Song, linux-kernel,
	linux-pci

Use devm_platform_ioremap_resource_byname() in order to simplify the
logic and to make the logic for Kirin 970 similar to the one for Kirin
960.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/pci/controller/dwc/pcie-kirin.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index f46a51ffd2c8..e1ebe0747978 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -297,13 +297,13 @@ static long kirin960_pcie_get_resource(struct kirin_pcie *kirin_pcie,
 {
 	struct device *dev = &pdev->dev;
 
-	kirin_pcie->apb_base =
-		devm_platform_ioremap_resource_byname(pdev, "apb");
+	kirin_pcie->apb_base = devm_platform_ioremap_resource_byname(pdev,
+								     "apb");
 	if (IS_ERR(kirin_pcie->apb_base))
 		return PTR_ERR(kirin_pcie->apb_base);
 
-	kirin_pcie->phy_base =
-		devm_platform_ioremap_resource_byname(pdev, "phy");
+	kirin_pcie->phy_base = devm_platform_ioremap_resource_byname(pdev,
+								     "phy");
 	if (IS_ERR(kirin_pcie->phy_base))
 		return PTR_ERR(kirin_pcie->phy_base);
 
@@ -329,23 +329,20 @@ static long kirin970_pcie_get_resource(struct kirin_pcie *kirin_pcie,
 				      struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct resource *apb;
-	struct resource *phy;
-	struct resource *dbi;
 	int ret;
 
-	apb = platform_get_resource_byname(pdev, IORESOURCE_MEM, "apb");
-	kirin_pcie->apb_base = devm_ioremap_resource(dev, apb);
+	kirin_pcie->apb_base = devm_platform_ioremap_resource_byname(pdev,
+								     "apb");
 	if (IS_ERR(kirin_pcie->apb_base))
 		return PTR_ERR(kirin_pcie->apb_base);
 
-	phy = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy");
-	kirin_pcie->phy_base = devm_ioremap_resource(dev, phy);
+	kirin_pcie->phy_base = devm_platform_ioremap_resource_byname(pdev,
+								     "phy");
 	if (IS_ERR(kirin_pcie->phy_base))
 		return PTR_ERR(kirin_pcie->phy_base);
 
-	dbi = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
-	kirin_pcie->pci->dbi_base = devm_ioremap_resource(dev, dbi);
+	kirin_pcie->pci->dbi_base = devm_platform_ioremap_resource_byname(pdev,
+									  "dbi");
 	if (IS_ERR(kirin_pcie->pci->dbi_base))
 		return PTR_ERR(kirin_pcie->pci->dbi_base);
 
-- 
2.29.2


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

* [PATCH v2 07/11] PCI: dwc: pcie-kirin: place common init code altogether
  2021-02-03  7:01 [PATCH v2 00/11] Add support for Hikey 970 PCIe Mauro Carvalho Chehab
                   ` (5 preceding siblings ...)
  2021-02-03  7:01 ` [PATCH v2 06/11] PCI: dwc: pcie-kirin: simplify Kirin 970 get resource logic Mauro Carvalho Chehab
@ 2021-02-03  7:01 ` Mauro Carvalho Chehab
  2021-02-03  7:01 ` [PATCH v2 08/11] PCI: dwc: pcie-kirin: add support for a regulator Mauro Carvalho Chehab
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 24+ messages in thread
From: Mauro Carvalho Chehab @ 2021-02-03  7:01 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Binghui Wang, Bjorn Helgaas,
	Lorenzo Pieralisi, Rob Herring, Xiaowei Song, linux-kernel,
	linux-pci

Both Kirin 960 and Kirin 970 need to do ioremap for apb, phy and dbi.

So, use a shared code for those.

It should be noticed that the dbi remap is now done by dwc core, so it
can simply be removed from kirin970_pcie_get_resource().

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/pci/controller/dwc/pcie-kirin.c | 45 +++++++++++--------------
 1 file changed, 20 insertions(+), 25 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index e1ebe0747978..2bce6e3750d4 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -292,21 +292,27 @@ static void kirin970_pcie_set_eyeparam(struct kirin_pcie *kirin_pcie)
 	kirin_apb_natural_phy_writel(kirin_pcie, val, LANEN_DIG_ASIC_TX_OVRD_IN_1);
 }
 
+static long kirin_common_pcie_get_resource(struct kirin_pcie *kirin_pcie,
+					   struct platform_device *pdev)
+{
+	kirin_pcie->apb_base = devm_platform_ioremap_resource_byname(pdev,
+								     "apb");
+	if (IS_ERR(kirin_pcie->apb_base))
+		return PTR_ERR(kirin_pcie->apb_base);
+
+	kirin_pcie->phy_base = devm_platform_ioremap_resource_byname(pdev,
+								     "phy");
+	if (IS_ERR(kirin_pcie->phy_base))
+		return PTR_ERR(kirin_pcie->phy_base);
+
+	return 0;
+}
+
 static long kirin960_pcie_get_resource(struct kirin_pcie *kirin_pcie,
 				       struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 
-	kirin_pcie->apb_base = devm_platform_ioremap_resource_byname(pdev,
-								     "apb");
-	if (IS_ERR(kirin_pcie->apb_base))
-		return PTR_ERR(kirin_pcie->apb_base);
-
-	kirin_pcie->phy_base = devm_platform_ioremap_resource_byname(pdev,
-								     "phy");
-	if (IS_ERR(kirin_pcie->phy_base))
-		return PTR_ERR(kirin_pcie->phy_base);
-
 	kirin_pcie->crgctrl =
 		syscon_regmap_lookup_by_compatible("hisilicon,hi3660-crgctrl");
 	if (IS_ERR(kirin_pcie->crgctrl))
@@ -331,21 +337,6 @@ static long kirin970_pcie_get_resource(struct kirin_pcie *kirin_pcie,
 	struct device *dev = &pdev->dev;
 	int ret;
 
-	kirin_pcie->apb_base = devm_platform_ioremap_resource_byname(pdev,
-								     "apb");
-	if (IS_ERR(kirin_pcie->apb_base))
-		return PTR_ERR(kirin_pcie->apb_base);
-
-	kirin_pcie->phy_base = devm_platform_ioremap_resource_byname(pdev,
-								     "phy");
-	if (IS_ERR(kirin_pcie->phy_base))
-		return PTR_ERR(kirin_pcie->phy_base);
-
-	kirin_pcie->pci->dbi_base = devm_platform_ioremap_resource_byname(pdev,
-									  "dbi");
-	if (IS_ERR(kirin_pcie->pci->dbi_base))
-		return PTR_ERR(kirin_pcie->pci->dbi_base);
-
 	kirin970_pcie_get_eyeparam(kirin_pcie);
 
 	kirin_pcie->gpio_id_reset[0] = of_get_named_gpio(dev->of_node,
@@ -1141,6 +1132,10 @@ static int kirin_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	ret = kirin_common_pcie_get_resource(kirin_pcie, pdev);
+	if (ret)
+		return ret;
+
 	ret = ops->get_resource(kirin_pcie, pdev);
 	if (ret)
 		return ret;
-- 
2.29.2


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

* [PATCH v2 08/11] PCI: dwc: pcie-kirin: add support for a regulator
  2021-02-03  7:01 [PATCH v2 00/11] Add support for Hikey 970 PCIe Mauro Carvalho Chehab
                   ` (6 preceding siblings ...)
  2021-02-03  7:01 ` [PATCH v2 07/11] PCI: dwc: pcie-kirin: place common init code altogether Mauro Carvalho Chehab
@ 2021-02-03  7:01 ` Mauro Carvalho Chehab
  2021-02-03  7:01 ` [PATCH v2 09/11] PCI: dwc: pcie-kirin: allow using multiple reset GPIOs Mauro Carvalho Chehab
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 24+ messages in thread
From: Mauro Carvalho Chehab @ 2021-02-03  7:01 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Binghui Wang, Bjorn Helgaas,
	Lorenzo Pieralisi, Rob Herring, Xiaowei Song, linux-kernel,
	linux-pci

On Kirin 970 designs, a power supply is required to enable
a PCI bridge and other components of the board.

For instance, on HiKey 970, the Hi6421v600 regulator provides a
power line (LDO33) which powers on the PCI bridge, the M.2 slot,
the mini PCIe 1x slot and the Realtek 8169 Ethernet card.

Without enabling such power supply, the PCI resource allocation
fails.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/pci/controller/dwc/pcie-kirin.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index 2bce6e3750d4..005fc4c2ad7f 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -22,6 +22,7 @@
 #include <linux/pci_regs.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
 #include <linux/resource.h>
 #include <linux/types.h>
 #include "pcie-designware.h"
@@ -335,8 +336,21 @@ static long kirin970_pcie_get_resource(struct kirin_pcie *kirin_pcie,
 				      struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct regulator *reg;
 	int ret;
 
+	reg = devm_regulator_get(dev, "pcie_vdd");
+	if (IS_ERR_OR_NULL(reg)) {
+		if (PTR_ERR(reg) == -EPROBE_DEFER)
+		    return PTR_ERR(reg);
+	} else {
+		ret = regulator_enable(reg);
+		if (ret) {
+			dev_err(dev, "Failed to enable regulator\n");
+			return ret;
+		}
+	}
+
 	kirin970_pcie_get_eyeparam(kirin_pcie);
 
 	kirin_pcie->gpio_id_reset[0] = of_get_named_gpio(dev->of_node,
-- 
2.29.2


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

* [PATCH v2 09/11] PCI: dwc: pcie-kirin: allow using multiple reset GPIOs
  2021-02-03  7:01 [PATCH v2 00/11] Add support for Hikey 970 PCIe Mauro Carvalho Chehab
                   ` (7 preceding siblings ...)
  2021-02-03  7:01 ` [PATCH v2 08/11] PCI: dwc: pcie-kirin: add support for a regulator Mauro Carvalho Chehab
@ 2021-02-03  7:01 ` Mauro Carvalho Chehab
  2021-02-03 13:46   ` Mark Brown
  2021-02-03 15:18   ` Krzysztof Wilczyński
  2021-02-03  7:01 ` [PATCH v2 10/11] PCI: dwc: pcie-kirin: add support for clkreq GPIOs Mauro Carvalho Chehab
  2021-02-03  7:01 ` [PATCH v2 11/11] pci: dwc: pcie-kirin: cleanup kirin970_pcie_get_eyeparam() Mauro Carvalho Chehab
  10 siblings, 2 replies; 24+ messages in thread
From: Mauro Carvalho Chehab @ 2021-02-03  7:01 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Binghui Wang, Bjorn Helgaas,
	Liam Girdwood, Lorenzo Pieralisi, Mark Brown, Rob Herring,
	Xiaowei Song, linux-kernel, linux-pci

On HiKey 970, the PCI hardware contains a bridge (PEX 8606), an Ethernet
controller (RTL8169), a M.2 connector and a mini 1X connector.

They work out of the box, but each of them requires its own reset line,
which should be initialized when the PCI hardware is reset.

So, add support for the DTS to contain multiple reset lines.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/pci/controller/dwc/pcie-kirin.c | 148 +++++++++++++-----------
 1 file changed, 79 insertions(+), 69 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index 005fc4c2ad7f..4124c6ace349 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -139,6 +139,7 @@
 #define TIME_PHY_PD_MIN		10
 #define TIME_PHY_PD_MAX		11
 
+#define MAX_GPIO_RESETS		4
 struct kirin_pcie {
 	struct dw_pcie	*pci;
 	void __iomem	*apb_base;
@@ -151,8 +152,10 @@ struct kirin_pcie {
 	struct clk	*phy_ref_clk;
 	struct clk	*pcie_aclk;
 	struct clk	*pcie_aux_clk;
-	int		gpio_id_reset[4];
+	int		n_gpio_resets;
 	int		gpio_id_clkreq[3];
+	int		gpio_id_reset[MAX_GPIO_RESETS];
+	const char	*reset_names[MAX_GPIO_RESETS];
 	u32		eye_param[5];
 };
 
@@ -296,6 +299,24 @@ static void kirin970_pcie_set_eyeparam(struct kirin_pcie *kirin_pcie)
 static long kirin_common_pcie_get_resource(struct kirin_pcie *kirin_pcie,
 					   struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct regulator *reg;
+	char name[32];
+	int ret, i;
+
+	reg = devm_regulator_get_optional(dev, "pci");
+	if (IS_ERR_OR_NULL(reg)) {
+		if (PTR_ERR(reg) == -EPROBE_DEFER)
+		    return PTR_ERR(reg);
+	} else {
+		ret = regulator_enable(reg);
+		if (ret) {
+			dev_err(dev, "Failed to enable regulator\n");
+			return ret;
+		}
+	}
+
 	kirin_pcie->apb_base = devm_platform_ioremap_resource_byname(pdev,
 								     "apb");
 	if (IS_ERR(kirin_pcie->apb_base))
@@ -306,14 +327,47 @@ static long kirin_common_pcie_get_resource(struct kirin_pcie *kirin_pcie,
 	if (IS_ERR(kirin_pcie->phy_base))
 		return PTR_ERR(kirin_pcie->phy_base);
 
+	kirin_pcie->n_gpio_resets = of_gpio_named_count(np, "reset-gpios");
+	if (kirin_pcie->n_gpio_resets > MAX_GPIO_RESETS) {
+		dev_err(dev, "Too many GPIO resets!\n");
+		return -EINVAL;
+	}
+	for (i = 0; i < kirin_pcie->n_gpio_resets; i++) {
+		kirin_pcie->gpio_id_reset[i] = of_get_named_gpio(dev->of_node,
+							    "reset-gpios", i);
+		if (kirin_pcie->gpio_id_reset[i] < 0)
+			return kirin_pcie->gpio_id_reset[i];
+
+		sprintf(name, "pcie_perst_%d", i);
+		kirin_pcie->reset_names[i] = devm_kstrdup_const(dev, name,
+								GFP_KERNEL);
+		if (!kirin_pcie->reset_names[i])
+			return -ENOMEM;
+	}
+
 	return 0;
 }
 
+static int kirin_gpio_request(struct kirin_pcie *kirin_pcie,
+			      struct device *dev)
+{
+	int ret, i;
+
+	for (i = 0; i < kirin_pcie->n_gpio_resets; i++) {
+		ret = devm_gpio_request(dev, kirin_pcie->gpio_id_reset[i],
+					kirin_pcie->reset_names[i]);
+		if (ret)
+			return ret;
+	}
+
+
+	return ret;
+}
+
+
 static long kirin960_pcie_get_resource(struct kirin_pcie *kirin_pcie,
 				       struct platform_device *pdev)
 {
-	struct device *dev = &pdev->dev;
-
 	kirin_pcie->crgctrl =
 		syscon_regmap_lookup_by_compatible("hisilicon,hi3660-crgctrl");
 	if (IS_ERR(kirin_pcie->crgctrl))
@@ -324,16 +378,11 @@ static long kirin960_pcie_get_resource(struct kirin_pcie *kirin_pcie,
 	if (IS_ERR(kirin_pcie->sysctrl))
 		return PTR_ERR(kirin_pcie->sysctrl);
 
-	kirin_pcie->gpio_id_reset[0] = of_get_named_gpio(dev->of_node,
-						      "reset-gpios", 0);
-	if (kirin_pcie->gpio_id_reset[0] < 0)
-		return kirin_pcie->gpio_id_reset[0];
-
 	return 0;
 }
 
 static long kirin970_pcie_get_resource(struct kirin_pcie *kirin_pcie,
-				      struct platform_device *pdev)
+				       struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct regulator *reg;
@@ -353,40 +402,7 @@ static long kirin970_pcie_get_resource(struct kirin_pcie *kirin_pcie,
 
 	kirin970_pcie_get_eyeparam(kirin_pcie);
 
-	kirin_pcie->gpio_id_reset[0] = of_get_named_gpio(dev->of_node,
-						"switch,reset-gpios", 0);
-	if (kirin_pcie->gpio_id_reset[0] < 0)
-		return kirin_pcie->gpio_id_reset[0];
-
-	kirin_pcie->gpio_id_reset[1] = of_get_named_gpio(dev->of_node,
-						"eth,reset-gpios", 0);
-	if (kirin_pcie->gpio_id_reset[1] < 0)
-		return kirin_pcie->gpio_id_reset[1];
-
-	kirin_pcie->gpio_id_reset[2] = of_get_named_gpio(dev->of_node,
-						"m_2,reset-gpios", 0);
-	if (kirin_pcie->gpio_id_reset[2] < 0)
-		return kirin_pcie->gpio_id_reset[2];
-
-	kirin_pcie->gpio_id_reset[3] = of_get_named_gpio(dev->of_node,
-						"mini1,reset-gpios", 0);
-	if (kirin_pcie->gpio_id_reset[3] < 0)
-		return kirin_pcie->gpio_id_reset[3];
-
-	ret = devm_gpio_request(dev, kirin_pcie->gpio_id_reset[0],
-				    "pcie_switch_reset");
-	if (ret)
-		return ret;
-	ret = devm_gpio_request(dev, kirin_pcie->gpio_id_reset[1],
-				    "pcie_eth_reset");
-	if (ret)
-		return ret;
-	ret = devm_gpio_request(dev, kirin_pcie->gpio_id_reset[2],
-				    "pcie_m_2_reset");
-	if (ret)
-		return ret;
-	ret = devm_gpio_request(dev, kirin_pcie->gpio_id_reset[3],
-				    "pcie_mini1_reset");
+	ret = kirin_gpio_request(kirin_pcie, dev);
 	if (ret)
 		return ret;
 
@@ -846,7 +862,7 @@ static int kirin970_pcie_noc_power(struct kirin_pcie *kirin_pcie, bool enable)
 static int kirin970_pcie_power_on(struct kirin_pcie *kirin_pcie)
 {
 	struct device *dev = kirin_pcie->pci->dev;
-	int ret;
+	int ret, i;
 	u32 val;
 
 	/* Power supply for Host */
@@ -898,22 +914,11 @@ static int kirin970_pcie_power_on(struct kirin_pcie *kirin_pcie)
 
 	/* perst assert Endpoints */
 	usleep_range(21000, 23000);
-	ret = gpio_direction_output(kirin_pcie->gpio_id_reset[0], 1);
-	if (ret)
-		goto close_clk;
-
-	ret = gpio_direction_output(kirin_pcie->gpio_id_reset[1], 1);
-	if (ret)
-		goto close_clk;
-
-	ret = gpio_direction_output(kirin_pcie->gpio_id_reset[2], 1);
-	if (ret)
-		goto close_clk;
-
-	ret = gpio_direction_output(kirin_pcie->gpio_id_reset[3], 1);
-	if (ret)
-		goto close_clk;
-
+	for (i = 0; i < kirin_pcie->n_gpio_resets; i++) {
+		ret = gpio_direction_output(kirin_pcie->gpio_id_reset[i], 1);
+		if (ret)
+			return ret;
+	}
 	usleep_range(10000, 11000);
 
 	ret = is_pipe_clk_stable(kirin_pcie);
@@ -934,6 +939,7 @@ static int kirin970_pcie_power_on(struct kirin_pcie *kirin_pcie)
 
 static int kirin960_pcie_power_on(struct kirin_pcie *kirin_pcie)
 {
+	struct device *dev = kirin_pcie->pci->dev;
 	int ret;
 
 	/* Power supply for Host */
@@ -959,15 +965,19 @@ static int kirin960_pcie_power_on(struct kirin_pcie *kirin_pcie)
 		goto close_clk;
 
 	/* perst assert Endpoint */
-	if (!gpio_request(kirin_pcie->gpio_id_reset[0], "pcie_perst")) {
-		usleep_range(REF_2_PERST_MIN, REF_2_PERST_MAX);
-		ret = gpio_direction_output(kirin_pcie->gpio_id_reset[0], 1);
-		if (ret)
-			goto close_clk;
-		usleep_range(PERST_2_ACCESS_MIN, PERST_2_ACCESS_MAX);
-
-		return 0;
-	}
+	ret = kirin_gpio_request(kirin_pcie, dev);
+	if (ret)
+		goto close_clk;
+
+	usleep_range(REF_2_PERST_MIN, REF_2_PERST_MAX);
+
+	ret = gpio_direction_output(kirin_pcie->gpio_id_reset[0], 1);
+	if (ret)
+		goto close_clk;
+
+	usleep_range(PERST_2_ACCESS_MIN, PERST_2_ACCESS_MAX);
+
+	return 0;
 
 close_clk:
 	kirin_pcie_clk_ctrl(kirin_pcie, false);
-- 
2.29.2


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

* [PATCH v2 10/11] PCI: dwc: pcie-kirin: add support for clkreq GPIOs
  2021-02-03  7:01 [PATCH v2 00/11] Add support for Hikey 970 PCIe Mauro Carvalho Chehab
                   ` (8 preceding siblings ...)
  2021-02-03  7:01 ` [PATCH v2 09/11] PCI: dwc: pcie-kirin: allow using multiple reset GPIOs Mauro Carvalho Chehab
@ 2021-02-03  7:01 ` Mauro Carvalho Chehab
  2021-02-03  7:01 ` [PATCH v2 11/11] pci: dwc: pcie-kirin: cleanup kirin970_pcie_get_eyeparam() Mauro Carvalho Chehab
  10 siblings, 0 replies; 24+ messages in thread
From: Mauro Carvalho Chehab @ 2021-02-03  7:01 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Binghui Wang, Bjorn Helgaas,
	Lorenzo Pieralisi, Rob Herring, Xiaowei Song, linux-kernel,
	linux-pci

The hardware on Kirin 970 designs use external components as part of
their PCIe hardware support.

Fo instance, in the case of HiKey 970, it has separate clkreq lines that
are needed to be enabled on its PCIe bridge, Ethernet chip and even at
the M.2 connector hardware.

Those should be enabled during PCIe hardware power on logic, as
otherwise the resource allocation will fail.

Add support for them.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/pci/controller/dwc/pcie-kirin.c | 77 +++++++++++--------------
 1 file changed, 34 insertions(+), 43 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index 4124c6ace349..c5404f1eca28 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -140,6 +140,8 @@
 #define TIME_PHY_PD_MAX		11
 
 #define MAX_GPIO_RESETS		4
+#define MAX_GPIO_CLKREQ		3
+
 struct kirin_pcie {
 	struct dw_pcie	*pci;
 	void __iomem	*apb_base;
@@ -153,9 +155,11 @@ struct kirin_pcie {
 	struct clk	*pcie_aclk;
 	struct clk	*pcie_aux_clk;
 	int		n_gpio_resets;
-	int		gpio_id_clkreq[3];
+	int		n_gpio_clkreq;
 	int		gpio_id_reset[MAX_GPIO_RESETS];
 	const char	*reset_names[MAX_GPIO_RESETS];
+	int		gpio_id_clkreq[MAX_GPIO_CLKREQ];
+	const char	*clkreq_names[MAX_GPIO_CLKREQ];
 	u32		eye_param[5];
 };
 
@@ -345,6 +349,24 @@ static long kirin_common_pcie_get_resource(struct kirin_pcie *kirin_pcie,
 			return -ENOMEM;
 	}
 
+	kirin_pcie->n_gpio_clkreq = of_gpio_named_count(np, "clkreq-gpios");
+	if (kirin_pcie->n_gpio_clkreq > MAX_GPIO_CLKREQ) {
+		dev_err(dev, "Too many GPIO clock requests!\n");
+		return -EINVAL;
+	}
+	for (i = 0; i < kirin_pcie->n_gpio_clkreq; i++) {
+		kirin_pcie->gpio_id_clkreq[i] = of_get_named_gpio(dev->of_node,
+							    "clkreq-gpios", i);
+		if (kirin_pcie->gpio_id_clkreq[i] < 0)
+			return kirin_pcie->gpio_id_clkreq[i];
+
+		sprintf(name, "pcie_clkreq_%d", i);
+		kirin_pcie->clkreq_names[i] = devm_kstrdup_const(dev, name,
+								GFP_KERNEL);
+		if (!kirin_pcie->clkreq_names[i])
+			return -ENOMEM;
+	}
+
 	return 0;
 }
 
@@ -360,6 +382,12 @@ static int kirin_gpio_request(struct kirin_pcie *kirin_pcie,
 			return ret;
 	}
 
+	for (i = 0; i < kirin_pcie->n_gpio_clkreq; i++) {
+		ret = devm_gpio_request(dev, kirin_pcie->gpio_id_clkreq[i],
+					kirin_pcie->clkreq_names[i]);
+		if (ret)
+			return ret;
+	}
 
 	return ret;
 }
@@ -406,36 +434,6 @@ static long kirin970_pcie_get_resource(struct kirin_pcie *kirin_pcie,
 	if (ret)
 		return ret;
 
-	kirin_pcie->gpio_id_clkreq[0] = of_get_named_gpio(dev->of_node,
-						"eth,clkreq-gpios", 0);
-	if (kirin_pcie->gpio_id_clkreq[0] < 0)
-		return -ENODEV;
-
-	kirin_pcie->gpio_id_clkreq[1] = of_get_named_gpio(dev->of_node,
-						"m_2,clkreq-gpios", 0);
-	if (kirin_pcie->gpio_id_clkreq[1] < 0)
-		return -ENODEV;
-
-	kirin_pcie->gpio_id_clkreq[2] = of_get_named_gpio(dev->of_node,
-						"mini1,clkreq-gpios", 0);
-	if (kirin_pcie->gpio_id_clkreq[2] < 0)
-		return -ENODEV;
-
-	ret = devm_gpio_request(dev, kirin_pcie->gpio_id_clkreq[0],
-				    "pcie_eth_clkreq");
-	if (ret)
-		return ret;
-
-	ret = devm_gpio_request(dev, kirin_pcie->gpio_id_clkreq[1],
-				    "pcie_m_2_clkreq");
-	if (ret)
-		return ret;
-
-	ret = devm_gpio_request(dev, kirin_pcie->gpio_id_clkreq[2],
-				    "pcie_mini1_clkreq");
-	if (ret)
-		return ret;
-
 	kirin_pcie->crgctrl =
 		syscon_regmap_lookup_by_compatible("hisilicon,hi3670-crgctrl");
 	if (IS_ERR(kirin_pcie->crgctrl))
@@ -861,7 +859,6 @@ static int kirin970_pcie_noc_power(struct kirin_pcie *kirin_pcie, bool enable)
 
 static int kirin970_pcie_power_on(struct kirin_pcie *kirin_pcie)
 {
-	struct device *dev = kirin_pcie->pci->dev;
 	int ret, i;
 	u32 val;
 
@@ -871,17 +868,11 @@ static int kirin970_pcie_power_on(struct kirin_pcie *kirin_pcie)
 	usleep_range(TIME_CMOS_MIN, TIME_CMOS_MAX);
 	kirin_pcie_oe_enable(kirin_pcie);
 
-	ret = gpio_direction_output(kirin_pcie->gpio_id_clkreq[0], 0);
-	if (ret)
-		dev_err(dev, "Failed to pulse eth clkreq signal\n");
-
-	ret = gpio_direction_output(kirin_pcie->gpio_id_clkreq[1], 0);
-	if (ret)
-		dev_err(dev, "Failed to pulse m.2 clkreq signal\n");
-
-	ret = gpio_direction_output(kirin_pcie->gpio_id_clkreq[2], 0);
-	if (ret)
-		dev_err(dev, "Failed to pulse mini1 clkreq signal\n");
+	for (i = 0; i < kirin_pcie->n_gpio_clkreq; i++) {
+		ret = gpio_direction_output(kirin_pcie->gpio_id_clkreq[i], 0);
+		if (ret)
+			return ret;
+	}
 
 	ret = kirin_pcie_clk_ctrl(kirin_pcie, true);
 	if (ret)
-- 
2.29.2


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

* [PATCH v2 11/11] pci: dwc: pcie-kirin: cleanup kirin970_pcie_get_eyeparam()
  2021-02-03  7:01 [PATCH v2 00/11] Add support for Hikey 970 PCIe Mauro Carvalho Chehab
                   ` (9 preceding siblings ...)
  2021-02-03  7:01 ` [PATCH v2 10/11] PCI: dwc: pcie-kirin: add support for clkreq GPIOs Mauro Carvalho Chehab
@ 2021-02-03  7:01 ` Mauro Carvalho Chehab
  10 siblings, 0 replies; 24+ messages in thread
From: Mauro Carvalho Chehab @ 2021-02-03  7:01 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Binghui Wang, Bjorn Helgaas,
	Lorenzo Pieralisi, Rob Herring, Xiaowei Song, linux-kernel,
	linux-pci

Cleanup the routine, to let it clearer that eye_param is optional and
that, if not specified, the driver will assume the default.

While here, also drop the useless debug prints.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/pci/controller/dwc/pcie-kirin.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index c5404f1eca28..88f10aff8b09 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -246,21 +246,18 @@ static long kirin_pcie_get_clk(struct kirin_pcie *kirin_pcie,
 void kirin970_pcie_get_eyeparam(struct kirin_pcie *pcie)
 {
 	struct device *dev = pcie->pci->dev;
-	int i;
 	struct device_node *np;
+	int ret, i;
 
 	np = dev->of_node;
 
-	if (of_property_read_u32_array(np, "eye_param", pcie->eye_param, 5)) {
-		for (i = 0; i < 5; i++)
+	ret = of_property_read_u32_array(np, "eye_param", pcie->eye_param, 5);
+	if (!ret)
+		return;
+
+	/* There's no optional eye_param property. Set array to default */
+	for (i = 0; i < 5; i++)
 		pcie->eye_param[i] = EYEPARAM_NOCFG;
-	}
-
-	dev_dbg(dev, "eye_param_vboost = [0x%x]\n", pcie->eye_param[0]);
-	dev_dbg(dev, "eye_param_iboost = [0x%x]\n", pcie->eye_param[1]);
-	dev_dbg(dev, "eye_param_pre = [0x%x]\n", pcie->eye_param[2]);
-	dev_dbg(dev, "eye_param_post = [0x%x]\n", pcie->eye_param[3]);
-	dev_dbg(dev, "eye_param_main = [0x%x]\n", pcie->eye_param[4]);
 }
 
 static void kirin970_pcie_set_eyeparam(struct kirin_pcie *kirin_pcie)
-- 
2.29.2


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

* Re: [PATCH v2 04/11] PCI: dwc: pcie-kirin: add support for Kirin 970 PCIe controller
  2021-02-03  7:01 ` [PATCH v2 04/11] PCI: dwc: pcie-kirin: add support for Kirin 970 PCIe controller Mauro Carvalho Chehab
@ 2021-02-03  9:32   ` kernel test robot
  2021-02-03 14:34   ` Rob Herring
  1 sibling, 0 replies; 24+ messages in thread
From: kernel test robot @ 2021-02-03  9:32 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: kbuild-all, linux-media, Manivannan Sadhasivam, Binghui Wang,
	Bjorn Helgaas, Lorenzo Pieralisi, Rob Herring, Xiaowei Song,
	linux-kernel, linux-pci, Mauro Carvalho Chehab

[-- Attachment #1: Type: text/plain, Size: 3004 bytes --]

Hi Mauro,

I love your patch! Perhaps something to improve:

[auto build test WARNING on pci/next]
[also build test WARNING on linus/master v5.11-rc6 next-20210125]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/Add-support-for-Hikey-970-PCIe/20210203-151220
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/6c6f3407849b681518423b465433a46482b78ebc
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Mauro-Carvalho-Chehab/Add-support-for-Hikey-970-PCIe/20210203-151220
        git checkout 6c6f3407849b681518423b465433a46482b78ebc
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=ia64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/pci/controller/dwc/pcie-kirin.c:238:6: warning: no previous prototype for 'kirin970_pcie_get_eyeparam' [-Wmissing-prototypes]
     238 | void kirin970_pcie_get_eyeparam(struct kirin_pcie *pcie)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for FRAME_POINTER
   Depends on DEBUG_KERNEL && (M68K || UML || SUPERH) || ARCH_WANT_FRAME_POINTERS
   Selected by
   - FAULT_INJECTION_STACKTRACE_FILTER && FAULT_INJECTION_DEBUG_FS && STACKTRACE_SUPPORT && !X86_64 && !MIPS && !PPC && !S390 && !MICROBLAZE && !ARM && !ARC && !X86


vim +/kirin970_pcie_get_eyeparam +238 drivers/pci/controller/dwc/pcie-kirin.c

   237	
 > 238	void kirin970_pcie_get_eyeparam(struct kirin_pcie *pcie)
   239	{
   240		struct device *dev = pcie->pci->dev;
   241		int i;
   242		struct device_node *np;
   243	
   244		np = dev->of_node;
   245	
   246		if (of_property_read_u32_array(np, "eye_param", pcie->eye_param, 5)) {
   247			for (i = 0; i < 5; i++)
   248			pcie->eye_param[i] = EYEPARAM_NOCFG;
   249		}
   250	
   251		dev_dbg(dev, "eye_param_vboost = [0x%x]\n", pcie->eye_param[0]);
   252		dev_dbg(dev, "eye_param_iboost = [0x%x]\n", pcie->eye_param[1]);
   253		dev_dbg(dev, "eye_param_pre = [0x%x]\n", pcie->eye_param[2]);
   254		dev_dbg(dev, "eye_param_post = [0x%x]\n", pcie->eye_param[3]);
   255		dev_dbg(dev, "eye_param_main = [0x%x]\n", pcie->eye_param[4]);
   256	}
   257	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 63467 bytes --]

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

* Re: [PATCH v2 09/11] PCI: dwc: pcie-kirin: allow using multiple reset GPIOs
  2021-02-03  7:01 ` [PATCH v2 09/11] PCI: dwc: pcie-kirin: allow using multiple reset GPIOs Mauro Carvalho Chehab
@ 2021-02-03 13:46   ` Mark Brown
  2021-02-03 17:28     ` Mauro Carvalho Chehab
  2021-02-03 15:18   ` Krzysztof Wilczyński
  1 sibling, 1 reply; 24+ messages in thread
From: Mark Brown @ 2021-02-03 13:46 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Binghui Wang, Bjorn Helgaas, Liam Girdwood, Lorenzo Pieralisi,
	Rob Herring, Xiaowei Song, linux-kernel, linux-pci

[-- Attachment #1: Type: text/plain, Size: 342 bytes --]

On Wed, Feb 03, 2021 at 08:01:53AM +0100, Mauro Carvalho Chehab wrote:

> +	reg = devm_regulator_get_optional(dev, "pci");
> +	if (IS_ERR_OR_NULL(reg)) {
> +		if (PTR_ERR(reg) == -EPROBE_DEFER)
> +		    return PTR_ERR(reg);
> +	} else {
> +		ret = regulator_enable(reg);

This is still misuse of regulator_get_optional() for the same reason.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH v2 04/11] PCI: dwc: pcie-kirin: add support for Kirin 970 PCIe controller
  2021-02-03  7:01 ` [PATCH v2 04/11] PCI: dwc: pcie-kirin: add support for Kirin 970 PCIe controller Mauro Carvalho Chehab
  2021-02-03  9:32   ` kernel test robot
@ 2021-02-03 14:34   ` Rob Herring
  2021-02-03 17:49     ` Mauro Carvalho Chehab
                       ` (2 more replies)
  1 sibling, 3 replies; 24+ messages in thread
From: Rob Herring @ 2021-02-03 14:34 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Manivannan Sadhasivam, Binghui Wang, Bjorn Helgaas,
	Lorenzo Pieralisi, Xiaowei Song, linux-kernel, PCI

On Wed, Feb 3, 2021 at 1:02 AM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>
> Add support for HiSilicon Kirin 970 (hi3670) SoC PCIe controller, based
> on Synopsys DesignWare PCIe controller IP.
>
> [mchehab+huawei@kernel.org: fix merge conflicts]
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>  drivers/pci/controller/dwc/pcie-kirin.c | 723 +++++++++++++++++++++++-
>  1 file changed, 707 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
> index 026fd1e42a55..5925d2b345a8 100644
> --- a/drivers/pci/controller/dwc/pcie-kirin.c
> +++ b/drivers/pci/controller/dwc/pcie-kirin.c
> @@ -29,6 +29,7 @@
>  #define to_kirin_pcie(x) dev_get_drvdata((x)->dev)
>
>  #define REF_CLK_FREQ                   100000000
> +#define AXI_CLK_FREQ                   207500000
>
>  /* PCIe ELBI registers */
>  #define SOC_PCIECTRL_CTRL0_ADDR                0x000
> @@ -60,6 +61,65 @@
>  #define PCIE_DEBOUNCE_PARAM    0xF0F400
>  #define PCIE_OE_BYPASS         (0x3 << 28)
>
> +/* PCIe CTRL registers */
> +#define SOC_PCIECTRL_CTRL0_ADDR   0x000
> +#define SOC_PCIECTRL_CTRL1_ADDR   0x004
> +#define SOC_PCIECTRL_CTRL7_ADDR   0x01c
> +#define SOC_PCIECTRL_CTRL12_ADDR  0x030
> +#define SOC_PCIECTRL_CTRL20_ADDR  0x050
> +#define SOC_PCIECTRL_CTRL21_ADDR  0x054
> +#define SOC_PCIECTRL_STATE0_ADDR  0x400
> +
> +/* PCIe PHY registers */
> +#define SOC_PCIEPHY_CTRL0_ADDR    0x000
> +#define SOC_PCIEPHY_CTRL1_ADDR    0x004
> +#define SOC_PCIEPHY_CTRL2_ADDR    0x008
> +#define SOC_PCIEPHY_CTRL3_ADDR    0x00c
> +#define SOC_PCIEPHY_CTRL38_ADDR   0x0098
> +#define SOC_PCIEPHY_STATE0_ADDR   0x400
> +
> +#define PCIE_LINKUP_ENABLE            (0x8020)
> +#define PCIE_ELBI_SLV_DBI_ENABLE      (0x1 << 21)
> +#define PCIE_LTSSM_ENABLE_BIT         (0x1 << 11)
> +#define PCIEPHY_RESET_BIT             (0x1 << 17)
> +#define PCIEPHY_PIPE_LINE0_RESET_BIT  (0x1 << 19)
> +
> +#define PORT_MSI_CTRL_ADDR            0x820
> +#define PORT_MSI_CTRL_UPPER_ADDR      0x824
> +#define PORT_MSI_CTRL_INT0_ENABLE     0x828

These are common DWC 'port logic' registers. I think the core DWC
should handle them if not already.

> +
> +#define EYEPARAM_NOCFG 0xFFFFFFFF
> +#define RAWLANEN_DIG_PCS_XF_TX_OVRD_IN_1 0x3001
> +#define SUP_DIG_LVL_OVRD_IN 0xf
> +#define LANEN_DIG_ASIC_TX_OVRD_IN_1 0x1002
> +#define LANEN_DIG_ASIC_TX_OVRD_IN_2 0x1003
> +
> +/* kirin970 pciephy register */
> +#define SOC_PCIEPHY_MMC1PLL_CTRL1  0xc04
> +#define SOC_PCIEPHY_MMC1PLL_CTRL16 0xC40
> +#define SOC_PCIEPHY_MMC1PLL_CTRL17 0xC44
> +#define SOC_PCIEPHY_MMC1PLL_CTRL20 0xC50
> +#define SOC_PCIEPHY_MMC1PLL_CTRL21 0xC54
> +#define SOC_PCIEPHY_MMC1PLL_STAT0  0xE00

This looks like it is almost all phy related. I think it should all be
moved to a separate phy driver. Yes, we have some other PCI drivers
controlling their phys directly where the phy registers are
intermingled with the PCI host registers, but I think those either
predate the phy subsystem or are really simple. I also have a dream to
move all the phy control to the DWC core code.

> +
> +#define CRGPERIPH_PEREN12   0x470
> +#define CRGPERIPH_PERDIS12  0x474
> +#define CRGPERIPH_PCIECTRL0 0x800
> +
> +/* define ie,oe cfg */
> +#define IO_IE_EN_HARD_BYPASS         (0x1 << 27)
> +#define IO_OE_EN_HARD_BYPASS         (0x1 << 11)
> +#define IO_HARD_CTRL_DEBOUNCE_BYPASS (0x1 << 10)
> +#define IO_OE_GT_MODE                (0x2 << 7)
> +#define DEBOUNCE_WAITCFG_IN          (0xf << 20)
> +#define DEBOUNCE_WAITCFG_OUT         (0xf << 13)
> +
> +/* noc power domain */
> +#define NOC_POWER_IDLEREQ_1 0x38c
> +#define NOC_POWER_IDLE_1    0x394
> +#define NOC_PW_MASK         0x10000
> +#define NOC_PW_SET_BIT      0x1
> +
>  /* peri_crg ctrl */
>  #define CRGCTRL_PCIE_ASSERT_OFFSET     0x88
>  #define CRGCTRL_PCIE_ASSERT_BIT                0x8c000000
> @@ -84,12 +144,21 @@ struct kirin_pcie {
>         void __iomem    *phy_base;
>         struct regmap   *crgctrl;
>         struct regmap   *sysctrl;
> +       struct regmap   *pmctrl;
>         struct clk      *apb_sys_clk;
>         struct clk      *apb_phy_clk;
>         struct clk      *phy_ref_clk;
>         struct clk      *pcie_aclk;
>         struct clk      *pcie_aux_clk;
> -       int             gpio_id_reset;
> +       int             gpio_id_reset[4];
> +       int             gpio_id_clkreq[3];
> +       u32             eye_param[5];
> +};
> +
> +struct kirin_pcie_ops {
> +       long (*get_resource)(struct kirin_pcie *kirin_pcie,
> +                            struct platform_device *pdev);
> +       int (*power_on)(struct kirin_pcie *kirin_pcie);

Never need to power off?

>  };
>
>  /* Registers in PCIeCTRL */
> @@ -116,6 +185,28 @@ static inline u32 kirin_apb_phy_readl(struct kirin_pcie *kirin_pcie, u32 reg)
>         return readl(kirin_pcie->phy_base + reg);
>  }
>
> +static inline void kirin970_apb_phy_writel(struct kirin_pcie *kirin_pcie,
> +                                       u32 val, u32 reg)
> +{
> +       writel(val, kirin_pcie->phy_base + 0x40000 + reg);

That definitely looks like the phy is a separate block.

> +}
> +
> +static inline u32 kirin970_apb_phy_readl(struct kirin_pcie *kirin_pcie, u32 reg)
> +{
> +       return readl(kirin_pcie->phy_base + 0x40000 + reg);
> +}
> +
> +static inline void kirin_apb_natural_phy_writel(struct kirin_pcie *kirin_pcie,
> +                                       u32 val, u32 reg)
> +{
> +       writel(val, kirin_pcie->phy_base + reg * 4);
> +}
> +
> +static inline u32 kirin_apb_natural_phy_readl(struct kirin_pcie *kirin_pcie, u32 reg)
> +{
> +       return readl(kirin_pcie->phy_base + reg * 4);
> +}
> +
>  static long kirin_pcie_get_clk(struct kirin_pcie *kirin_pcie,
>                                struct platform_device *pdev)
>  {
> @@ -144,9 +235,68 @@ static long kirin_pcie_get_clk(struct kirin_pcie *kirin_pcie,
>         return 0;
>  }
>
> -static long kirin_pcie_get_resource(struct kirin_pcie *kirin_pcie,
> -                                   struct platform_device *pdev)
> +void kirin970_pcie_get_eyeparam(struct kirin_pcie *pcie)
>  {
> +       struct device *dev = pcie->pci->dev;
> +       int i;
> +       struct device_node *np;
> +
> +       np = dev->of_node;
> +
> +       if (of_property_read_u32_array(np, "eye_param", pcie->eye_param, 5)) {
> +               for (i = 0; i < 5; i++)
> +               pcie->eye_param[i] = EYEPARAM_NOCFG;
> +       }
> +
> +       dev_dbg(dev, "eye_param_vboost = [0x%x]\n", pcie->eye_param[0]);
> +       dev_dbg(dev, "eye_param_iboost = [0x%x]\n", pcie->eye_param[1]);
> +       dev_dbg(dev, "eye_param_pre = [0x%x]\n", pcie->eye_param[2]);
> +       dev_dbg(dev, "eye_param_post = [0x%x]\n", pcie->eye_param[3]);
> +       dev_dbg(dev, "eye_param_main = [0x%x]\n", pcie->eye_param[4]);
> +}
> +
> +static void kirin970_pcie_set_eyeparam(struct kirin_pcie *kirin_pcie)
> +{
> +       u32 val;
> +
> +       val = kirin_apb_natural_phy_readl(kirin_pcie, RAWLANEN_DIG_PCS_XF_TX_OVRD_IN_1);
> +
> +       if (kirin_pcie->eye_param[1] != EYEPARAM_NOCFG) {
> +               val &= (~0xf00);
> +               val |= (kirin_pcie->eye_param[1] << 8) | (0x1 << 12);
> +       }
> +       kirin_apb_natural_phy_writel(kirin_pcie, val, RAWLANEN_DIG_PCS_XF_TX_OVRD_IN_1);
> +
> +       val = kirin_apb_natural_phy_readl(kirin_pcie, LANEN_DIG_ASIC_TX_OVRD_IN_2);
> +       val &= (~0x1FBF);
> +       if (kirin_pcie->eye_param[2] != EYEPARAM_NOCFG)
> +               val |= (kirin_pcie->eye_param[2]<< 0) | (0x1 << 6);
> +
> +       if (kirin_pcie->eye_param[3] != EYEPARAM_NOCFG)
> +               val |= (kirin_pcie->eye_param[3] << 7) | (0x1 << 13);
> +
> +       kirin_apb_natural_phy_writel(kirin_pcie, val, LANEN_DIG_ASIC_TX_OVRD_IN_2);
> +
> +       val = kirin_apb_natural_phy_readl(kirin_pcie, SUP_DIG_LVL_OVRD_IN);
> +       if (kirin_pcie->eye_param[0] != EYEPARAM_NOCFG) {
> +               val &= (~0x1C0);
> +               val |= (kirin_pcie->eye_param[0] << 6) | (0x1 << 9);
> +       }
> +       kirin_apb_natural_phy_writel(kirin_pcie, val, SUP_DIG_LVL_OVRD_IN);
> +
> +       val = kirin_apb_natural_phy_readl(kirin_pcie, LANEN_DIG_ASIC_TX_OVRD_IN_1);
> +       if (kirin_pcie->eye_param[4] != EYEPARAM_NOCFG) {
> +               val &= (~0x7E00);
> +               val |= (kirin_pcie->eye_param[4] << 9) | (0x1 << 15);
> +       }
> +       kirin_apb_natural_phy_writel(kirin_pcie, val, LANEN_DIG_ASIC_TX_OVRD_IN_1);
> +}
> +
> +static long kirin960_pcie_get_resource(struct kirin_pcie *kirin_pcie,
> +                                      struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +
>         kirin_pcie->apb_base =
>                 devm_platform_ioremap_resource_byname(pdev, "apb");
>         if (IS_ERR(kirin_pcie->apb_base))
> @@ -167,6 +317,122 @@ static long kirin_pcie_get_resource(struct kirin_pcie *kirin_pcie,
>         if (IS_ERR(kirin_pcie->sysctrl))
>                 return PTR_ERR(kirin_pcie->sysctrl);
>
> +       kirin_pcie->gpio_id_reset[0] = of_get_named_gpio(dev->of_node,
> +                                                     "reset-gpios", 0);
> +       if (kirin_pcie->gpio_id_reset[0] < 0)
> +               return -ENODEV;
> +
> +       return 0;
> +}
> +
> +static long kirin970_pcie_get_resource(struct kirin_pcie *kirin_pcie,
> +                                     struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct resource *apb;
> +       struct resource *phy;
> +       struct resource *dbi;
> +       int ret;
> +
> +       apb = platform_get_resource_byname(pdev, IORESOURCE_MEM, "apb");
> +       kirin_pcie->apb_base = devm_ioremap_resource(dev, apb);
> +       if (IS_ERR(kirin_pcie->apb_base))
> +               return PTR_ERR(kirin_pcie->apb_base);
> +
> +       phy = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy");
> +       kirin_pcie->phy_base = devm_ioremap_resource(dev, phy);
> +       if (IS_ERR(kirin_pcie->phy_base))
> +               return PTR_ERR(kirin_pcie->phy_base);
> +
> +       dbi = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
> +       kirin_pcie->pci->dbi_base = devm_ioremap_resource(dev, dbi);

The DWC core handles this now.

> +       if (IS_ERR(kirin_pcie->pci->dbi_base))
> +               return PTR_ERR(kirin_pcie->pci->dbi_base);
> +
> +       kirin970_pcie_get_eyeparam(kirin_pcie);
> +
> +       kirin_pcie->gpio_id_reset[0] = of_get_named_gpio(dev->of_node,
> +                                               "switch,reset-gpios", 0);
> +       if (kirin_pcie->gpio_id_reset[0] < 0)
> +               return -ENODEV;
> +
> +       kirin_pcie->gpio_id_reset[1] = of_get_named_gpio(dev->of_node,
> +                                               "eth,reset-gpios", 0);
> +       if (kirin_pcie->gpio_id_reset[1] < 0)
> +               return -ENODEV;
> +
> +       kirin_pcie->gpio_id_reset[2] = of_get_named_gpio(dev->of_node,
> +                                               "m_2,reset-gpios", 0);
> +       if (kirin_pcie->gpio_id_reset[2] < 0)
> +               return -ENODEV;
> +
> +       kirin_pcie->gpio_id_reset[3] = of_get_named_gpio(dev->of_node,
> +                                               "mini1,reset-gpios", 0);
> +       if (kirin_pcie->gpio_id_reset[3] < 0)

I've already explained how all this is wrong.

> +               return -ENODEV;
> +
> +       ret = devm_gpio_request(dev, kirin_pcie->gpio_id_reset[0],
> +                                   "pcie_switch_reset");
> +       if (ret)
> +               return ret;
> +       ret = devm_gpio_request(dev, kirin_pcie->gpio_id_reset[1],
> +                                   "pcie_eth_reset");
> +       if (ret)
> +               return ret;
> +       ret = devm_gpio_request(dev, kirin_pcie->gpio_id_reset[2],
> +                                   "pcie_m_2_reset");
> +       if (ret)
> +               return ret;
> +       ret = devm_gpio_request(dev, kirin_pcie->gpio_id_reset[3],
> +                                   "pcie_mini1_reset");
> +       if (ret)
> +               return ret;
> +
> +       kirin_pcie->gpio_id_clkreq[0] = of_get_named_gpio(dev->of_node,
> +                                               "eth,clkreq-gpios", 0);
> +       if (kirin_pcie->gpio_id_clkreq[0] < 0)
> +               return -ENODEV;
> +
> +       kirin_pcie->gpio_id_clkreq[1] = of_get_named_gpio(dev->of_node,
> +                                               "m_2,clkreq-gpios", 0);
> +       if (kirin_pcie->gpio_id_clkreq[1] < 0)
> +               return -ENODEV;
> +
> +       kirin_pcie->gpio_id_clkreq[2] = of_get_named_gpio(dev->of_node,
> +                                               "mini1,clkreq-gpios", 0);
> +       if (kirin_pcie->gpio_id_clkreq[2] < 0)
> +               return -ENODEV;
> +
> +       ret = devm_gpio_request(dev, kirin_pcie->gpio_id_clkreq[0],
> +                                   "pcie_eth_clkreq");
> +       if (ret)
> +               return ret;
> +
> +       ret = devm_gpio_request(dev, kirin_pcie->gpio_id_clkreq[1],
> +                                   "pcie_m_2_clkreq");
> +       if (ret)
> +               return ret;
> +
> +       ret = devm_gpio_request(dev, kirin_pcie->gpio_id_clkreq[2],
> +                                   "pcie_mini1_clkreq");
> +       if (ret)
> +               return ret;
> +
> +       kirin_pcie->crgctrl =
> +               syscon_regmap_lookup_by_compatible("hisilicon,hi3670-crgctrl");
> +       if (IS_ERR(kirin_pcie->crgctrl))
> +               return PTR_ERR(kirin_pcie->crgctrl);
> +
> +       kirin_pcie->sysctrl =
> +               syscon_regmap_lookup_by_compatible("hisilicon,hi3670-sctrl");
> +       if (IS_ERR(kirin_pcie->sysctrl))
> +               return PTR_ERR(kirin_pcie->sysctrl);
> +
> +       kirin_pcie->pmctrl =
> +               syscon_regmap_lookup_by_compatible("hisilicon,hi3670-pmctrl");
> +       if (IS_ERR(kirin_pcie->sysctrl))
> +               return PTR_ERR(kirin_pcie->sysctrl);
> +
>         return 0;
>  }
>
> @@ -208,6 +474,21 @@ static void kirin_pcie_oe_enable(struct kirin_pcie *kirin_pcie)
>         regmap_write(kirin_pcie->sysctrl, SCTRL_PCIE_OE_OFFSET, val);
>  }
>
> +static int kirin970_pcie_clk_ctrl(struct clk *clk, int clk_on)
> +{
> +       int ret = 0;
> +
> +       if (clk_on) {
> +               ret = clk_prepare_enable(clk);
> +               if (ret)
> +                       return ret;
> +       } else {
> +               clk_disable_unprepare(clk);
> +       }
> +
> +       return ret;
> +}
> +
>  static int kirin_pcie_clk_ctrl(struct kirin_pcie *kirin_pcie, bool enable)
>  {
>         int ret = 0;
> @@ -255,7 +536,401 @@ static int kirin_pcie_clk_ctrl(struct kirin_pcie *kirin_pcie, bool enable)
>         return ret;
>  }
>
> -static int kirin_pcie_power_on(struct kirin_pcie *kirin_pcie)
> +static void kirin970_pcie_natural_cfg(struct kirin_pcie *kirin_pcie)
> +{
> +       u32 val;
> +
> +       /* change 2p mem_ctrl */
> +       kirin_apb_ctrl_writel(kirin_pcie, 0x02605550, SOC_PCIECTRL_CTRL20_ADDR);
> +
> +       /* pull up sys_aux_pwr_det */
> +       val = kirin_apb_ctrl_readl(kirin_pcie, SOC_PCIECTRL_CTRL7_ADDR);
> +       val |= (0x1 << 10);
> +       kirin_apb_ctrl_writel(kirin_pcie, val, SOC_PCIECTRL_CTRL7_ADDR);
> +
> +       /* output, pull down */
> +       val = kirin_apb_ctrl_readl(kirin_pcie, SOC_PCIECTRL_CTRL12_ADDR);
> +       val &= ~(0x3 << 2);
> +       val |= (0x1 << 1);
> +       val &= ~(0x1 << 0);
> +       kirin_apb_ctrl_writel(kirin_pcie, val, SOC_PCIECTRL_CTRL12_ADDR);
> +
> +       /* Handle phy_reset and lane0_reset to HW */
> +       val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_CTRL1_ADDR);
> +       val |= PCIEPHY_RESET_BIT;
> +       val &= ~PCIEPHY_PIPE_LINE0_RESET_BIT;
> +       kirin970_apb_phy_writel(kirin_pcie, val, SOC_PCIEPHY_CTRL1_ADDR);
> +
> +       /* fix chip bug: TxDetectRx fail */
> +       val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_CTRL38_ADDR);
> +       val |= (0x1 << 2);
> +       kirin970_apb_phy_writel(kirin_pcie, val, SOC_PCIEPHY_CTRL38_ADDR);
> +}
> +
> +static void kirin970_pcie_pll_init(struct kirin_pcie *kirin_pcie)
> +{
> +       u32 val;
> +
> +       /* choose FNPLL */
> +       val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_MMC1PLL_CTRL1);
> +       val |= (0x1 << 27);
> +       kirin970_apb_phy_writel(kirin_pcie, val, SOC_PCIEPHY_MMC1PLL_CTRL1);
> +
> +       val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_MMC1PLL_CTRL16);
> +       val &= 0xF000FFFF;
> +       /* fnpll fbdiv = 0xD0 */
> +       val |= (0xd0 << 16);
> +       kirin970_apb_phy_writel(kirin_pcie, val, SOC_PCIEPHY_MMC1PLL_CTRL16);
> +
> +       val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_MMC1PLL_CTRL17);
> +       val &= 0xFF000000;
> +       /* fnpll fracdiv = 0x555555 */
> +       val |= (0x555555 << 0);
> +       kirin970_apb_phy_writel(kirin_pcie, val, SOC_PCIEPHY_MMC1PLL_CTRL17);
> +
> +       val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_MMC1PLL_CTRL20);
> +       val &= 0xF5FF88FF;
> +       /* fnpll dll_en = 0x1 */
> +       val |= (0x1 << 27);
> +       /* fnpll postdiv1 = 0x5 */
> +       val |= (0x5 << 8);
> +       /* fnpll postdiv2 = 0x4 */
> +       val |= (0x4 << 12);
> +       /* fnpll pll_mode = 0x0 */
> +       val &= ~(0x1 << 25);
> +       kirin970_apb_phy_writel(kirin_pcie, val, SOC_PCIEPHY_MMC1PLL_CTRL20);
> +
> +       kirin970_apb_phy_writel(kirin_pcie, 0x20, SOC_PCIEPHY_MMC1PLL_CTRL21);
> +}
> +
> +static int kirin970_pcie_pll_ctrl(struct kirin_pcie *kirin_pcie, bool enable)
> +{
> +       struct device *dev = kirin_pcie->pci->dev;
> +       u32 val;
> +       int time = 200;
> +
> +       if (enable) {
> +               /* pd = 0 */
> +               val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_MMC1PLL_CTRL16);
> +               val &= ~(0x1 << 0);
> +               kirin970_apb_phy_writel(kirin_pcie, val, SOC_PCIEPHY_MMC1PLL_CTRL16);
> +
> +               val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_MMC1PLL_STAT0);
> +
> +               /* choose FNPLL */
> +               while (!(val & 0x10)) {
> +                       if (!time) {
> +                               dev_err(dev, "wait for pll_lock timeout\n");
> +                               return -1;
> +                       }
> +                       time --;
> +                       udelay(1);
> +                       val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_MMC1PLL_STAT0);
> +               }
> +
> +               /* pciepll_bp = 0 */
> +               val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_MMC1PLL_CTRL20);
> +               val &= ~(0x1 << 16);
> +               kirin970_apb_phy_writel(kirin_pcie, val, SOC_PCIEPHY_MMC1PLL_CTRL20);
> +
> +       } else {
> +               /* pd = 1 */
> +               val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_MMC1PLL_CTRL16);
> +               val |= (0x1 << 0);
> +               kirin970_apb_phy_writel(kirin_pcie, val, SOC_PCIEPHY_MMC1PLL_CTRL16);
> +
> +               /* pciepll_bp = 1 */
> +               val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_MMC1PLL_CTRL20);
> +               val |= (0x1 << 16);
> +               kirin970_apb_phy_writel(kirin_pcie, val, SOC_PCIEPHY_MMC1PLL_CTRL20);
> +       }
> +
> +        return 0;
> +}
> +
> +static void kirin970_pcie_hp_debounce_gt(struct kirin_pcie *kirin_pcie, bool open)
> +{
> +       if (open)
> +               /* gt_clk_pcie_hp/gt_clk_pcie_debounce open */
> +               regmap_write(kirin_pcie->crgctrl, CRGPERIPH_PEREN12, 0x9000);
> +       else
> +               /* gt_clk_pcie_hp/gt_clk_pcie_debounce close */
> +               regmap_write(kirin_pcie->crgctrl, CRGPERIPH_PERDIS12, 0x9000);
> +}
> +
> +static void kirin970_pcie_phyref_gt(struct kirin_pcie *kirin_pcie, bool open)
> +{
> +       unsigned int val;
> +
> +       regmap_read(kirin_pcie->crgctrl, CRGPERIPH_PCIECTRL0, &val);
> +
> +       if (open)
> +               val &= ~(0x1 << 1); //enable hard gt mode
> +       else
> +               val |= (0x1 << 1); //disable hard gt mode
> +
> +       regmap_write(kirin_pcie->crgctrl, CRGPERIPH_PCIECTRL0, val);
> +
> +       /* disable soft gt mode */
> +       regmap_write(kirin_pcie->crgctrl, CRGPERIPH_PERDIS12, 0x4000);
> +}
> +
> +static void kirin970_pcie_oe_ctrl(struct kirin_pcie *kirin_pcie, bool en_flag)
> +{
> +       unsigned int val;
> +
> +       regmap_read(kirin_pcie->crgctrl , CRGPERIPH_PCIECTRL0, &val);
> +
> +       /* set ie cfg */
> +       val |= IO_IE_EN_HARD_BYPASS;
> +
> +       /* set oe cfg */
> +       val &= ~IO_HARD_CTRL_DEBOUNCE_BYPASS;
> +
> +       /* set phy_debounce in&out time */
> +       val |= (DEBOUNCE_WAITCFG_IN | DEBOUNCE_WAITCFG_OUT);
> +
> +       /* select oe_gt_mode */
> +       val |= IO_OE_GT_MODE;
> +
> +       if (en_flag)
> +               val &= ~IO_OE_EN_HARD_BYPASS;
> +       else
> +               val |= IO_OE_EN_HARD_BYPASS;
> +
> +       regmap_write(kirin_pcie->crgctrl, CRGPERIPH_PCIECTRL0, val);
> +}
> +
> +static void kirin970_pcie_ioref_gt(struct kirin_pcie *kirin_pcie, bool open)
> +{
> +       unsigned int val;
> +
> +       if (open) {
> +               kirin_apb_ctrl_writel(kirin_pcie, 0x20000070, SOC_PCIECTRL_CTRL21_ADDR);
> +
> +               kirin970_pcie_oe_ctrl(kirin_pcie, true);
> +
> +               /* en hard gt mode */
> +               regmap_read(kirin_pcie->crgctrl, CRGPERIPH_PCIECTRL0, &val);
> +               val &= ~(0x1 << 0);
> +               regmap_write(kirin_pcie->crgctrl, CRGPERIPH_PCIECTRL0, val);
> +
> +               /* disable soft gt mode */
> +               regmap_write(kirin_pcie->crgctrl, CRGPERIPH_PERDIS12, 0x2000);
> +
> +       } else {
> +               /* disable hard gt mode */
> +               regmap_read(kirin_pcie->crgctrl, CRGPERIPH_PCIECTRL0, &val);
> +               val |= (0x1 << 0);
> +               regmap_write(kirin_pcie->crgctrl, CRGPERIPH_PCIECTRL0, val);
> +
> +               /* disable soft gt mode */
> +               regmap_write(kirin_pcie->crgctrl, CRGPERIPH_PERDIS12, 0x2000);
> +
> +               kirin970_pcie_oe_ctrl(kirin_pcie, false);
> +       }
> +}
> +
> +static int kirin970_pcie_allclk_ctrl(struct kirin_pcie *kirin_pcie, bool clk_on)
> +{
> +       struct device *dev = kirin_pcie->pci->dev;
> +       u32 val;
> +       int ret = 0;
> +
> +       if (!clk_on)
> +               goto ALL_CLOSE;
> +
> +       /* choose 100MHz clk src: Bit[8]==1 pad, Bit[8]==0 pll */
> +       val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_CTRL1_ADDR);
> +       val &= ~(0x1 << 8);
> +       kirin970_apb_phy_writel(kirin_pcie, val, SOC_PCIEPHY_CTRL1_ADDR);
> +
> +       kirin970_pcie_pll_init(kirin_pcie);
> +
> +       ret = kirin970_pcie_pll_ctrl(kirin_pcie, true);
> +       if (ret) {
> +               dev_err(dev, "Failed to enable pll\n");
> +               return -1;
> +       }
> +       kirin970_pcie_hp_debounce_gt(kirin_pcie, true);
> +       kirin970_pcie_phyref_gt(kirin_pcie, true);
> +       kirin970_pcie_ioref_gt(kirin_pcie, true);
> +
> +       ret = clk_set_rate(kirin_pcie->pcie_aclk, AXI_CLK_FREQ);
> +       if (ret) {
> +               dev_err(dev, "Failed to set rate\n");
> +               goto GT_CLOSE;
> +       }
> +
> +       ret = kirin970_pcie_clk_ctrl(kirin_pcie->pcie_aclk, true);
> +       if (ret) {
> +               dev_err(dev, "Failed to enable pcie_aclk\n");
> +               goto GT_CLOSE;
> +       }
> +
> +       ret = kirin970_pcie_clk_ctrl(kirin_pcie->pcie_aux_clk, true);
> +       if (ret) {
> +               dev_err(dev, "Failed to enable pcie_aux_clk\n");
> +               goto AUX_CLK_FAIL;
> +       }
> +
> +       return 0;
> +
> +ALL_CLOSE:
> +       kirin970_pcie_clk_ctrl(kirin_pcie->pcie_aux_clk, false);
> +AUX_CLK_FAIL:
> +       kirin970_pcie_clk_ctrl(kirin_pcie->pcie_aclk, false);
> +GT_CLOSE:
> +       kirin970_pcie_ioref_gt(kirin_pcie, false);
> +       kirin970_pcie_phyref_gt(kirin_pcie, false);
> +       kirin970_pcie_hp_debounce_gt(kirin_pcie, false);
> +
> +       kirin970_pcie_pll_ctrl(kirin_pcie, false);
> +
> +       return ret;
> +}
> +
> +static bool is_pipe_clk_stable(struct kirin_pcie *kirin_pcie)
> +{
> +       struct device *dev = kirin_pcie->pci->dev;
> +       u32 val;
> +       u32 time = 100;
> +       u32 pipe_clk_stable = 0x1 << 19;
> +
> +       val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_STATE0_ADDR);
> +       while (val & pipe_clk_stable) {
> +               mdelay(1);
> +               if (time == 0) {
> +                       dev_err(dev, "PIPE clk is not stable\n");
> +                       return false;
> +               }
> +               time--;
> +               val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_STATE0_ADDR);
> +       }
> +
> +       return true;
> +}
> +
> +static int kirin970_pcie_noc_power(struct kirin_pcie *kirin_pcie, bool enable)
> +{
> +       struct device *dev = kirin_pcie->pci->dev;
> +       u32 time = 100;
> +       unsigned int val = NOC_PW_MASK;
> +       int rst;
> +
> +       if (enable)
> +               val = NOC_PW_MASK | NOC_PW_SET_BIT;
> +       else
> +               val = NOC_PW_MASK;
> +       rst = enable ? 1 : 0;
> +
> +       regmap_write(kirin_pcie->pmctrl, NOC_POWER_IDLEREQ_1, val);
> +
> +       time = 100;
> +       regmap_read(kirin_pcie->pmctrl, NOC_POWER_IDLE_1, &val);
> +       while((val & NOC_PW_SET_BIT) != rst) {
> +               udelay(10);
> +               if (!time) {
> +                       dev_err(dev, "Failed to reverse noc power-status\n");
> +                       return -1;
> +               }
> +               time--;
> +               regmap_read(kirin_pcie->pmctrl, NOC_POWER_IDLE_1, &val);
> +       }
> +
> +       return 0;
> +}
> +
> +static int kirin970_pcie_power_on(struct kirin_pcie *kirin_pcie)
> +{
> +       struct device *dev = kirin_pcie->pci->dev;
> +       int ret;
> +       u32 val;
> +
> +       /* Power supply for Host */
> +       regmap_write(kirin_pcie->sysctrl,
> +                    SCTRL_PCIE_CMOS_OFFSET, SCTRL_PCIE_CMOS_BIT);
> +       usleep_range(TIME_CMOS_MIN, TIME_CMOS_MAX);
> +       kirin_pcie_oe_enable(kirin_pcie);
> +
> +       ret = gpio_direction_output(kirin_pcie->gpio_id_clkreq[0], 0);
> +       if (ret)
> +               dev_err(dev, "Failed to pulse eth clkreq signal\n");
> +
> +       ret = gpio_direction_output(kirin_pcie->gpio_id_clkreq[1], 0);
> +       if (ret)
> +               dev_err(dev, "Failed to pulse m.2 clkreq signal\n");
> +
> +       ret = gpio_direction_output(kirin_pcie->gpio_id_clkreq[2], 0);
> +       if (ret)
> +               dev_err(dev, "Failed to pulse mini1 clkreq signal\n");
> +
> +       ret = kirin_pcie_clk_ctrl(kirin_pcie, true);
> +       if (ret)
> +               return ret;
> +
> +       /* ISO disable, PCIeCtrl, PHY assert and clk gate clear */
> +       regmap_write(kirin_pcie->sysctrl,
> +                    SCTRL_PCIE_ISO_OFFSET, SCTRL_PCIE_ISO_BIT);
> +       regmap_write(kirin_pcie->crgctrl,
> +                    CRGCTRL_PCIE_ASSERT_OFFSET, CRGCTRL_PCIE_ASSERT_BIT);
> +       regmap_write(kirin_pcie->sysctrl,
> +                    SCTRL_PCIE_HPCLK_OFFSET, SCTRL_PCIE_HPCLK_BIT);
> +
> +       kirin970_pcie_natural_cfg(kirin_pcie);
> +
> +       ret = kirin970_pcie_allclk_ctrl(kirin_pcie, true);
> +       if (ret)
> +               goto close_clk;
> +
> +       /* pull down phy_test_powerdown signal */
> +       val = kirin970_apb_phy_readl(kirin_pcie, SOC_PCIEPHY_CTRL0_ADDR);
> +       val &= ~(0x1 << 22);
> +       kirin970_apb_phy_writel(kirin_pcie, val, SOC_PCIEPHY_CTRL0_ADDR);
> +
> +       /* deassert controller perst_n */
> +       val = kirin_apb_ctrl_readl(kirin_pcie, SOC_PCIECTRL_CTRL12_ADDR);
> +       val |= (0x1 << 2);
> +       kirin_apb_ctrl_writel(kirin_pcie, val, SOC_PCIECTRL_CTRL12_ADDR);
> +       udelay(10);
> +
> +       /* perst assert Endpoints */
> +       usleep_range(21000, 23000);
> +       ret = gpio_direction_output(kirin_pcie->gpio_id_reset[0], 1);
> +       if (ret)
> +               goto close_clk;
> +
> +       ret = gpio_direction_output(kirin_pcie->gpio_id_reset[1], 1);
> +       if (ret)
> +               goto close_clk;
> +
> +       ret = gpio_direction_output(kirin_pcie->gpio_id_reset[2], 1);
> +       if (ret)
> +               goto close_clk;
> +
> +       ret = gpio_direction_output(kirin_pcie->gpio_id_reset[3], 1);
> +       if (ret)
> +               goto close_clk;
> +
> +       usleep_range(10000, 11000);
> +
> +       ret = is_pipe_clk_stable(kirin_pcie);
> +       if (!ret)
> +               goto close_clk;
> +
> +       kirin970_pcie_set_eyeparam(kirin_pcie);
> +
> +       ret = kirin970_pcie_noc_power(kirin_pcie, false);
> +       if (ret)
> +               goto close_clk;
> +
> +       return 0;
> +close_clk:
> +       kirin_pcie_clk_ctrl(kirin_pcie, false);
> +       return ret;
> +}
> +
> +static int kirin960_pcie_power_on(struct kirin_pcie *kirin_pcie)
>  {
>         int ret;
>
> @@ -282,9 +957,9 @@ static int kirin_pcie_power_on(struct kirin_pcie *kirin_pcie)
>                 goto close_clk;
>
>         /* perst assert Endpoint */
> -       if (!gpio_request(kirin_pcie->gpio_id_reset, "pcie_perst")) {
> +       if (!gpio_request(kirin_pcie->gpio_id_reset[0], "pcie_perst")) {
>                 usleep_range(REF_2_PERST_MIN, REF_2_PERST_MAX);
> -               ret = gpio_direction_output(kirin_pcie->gpio_id_reset, 1);
> +               ret = gpio_direction_output(kirin_pcie->gpio_id_reset[0], 1);
>                 if (ret)
>                         goto close_clk;
>                 usleep_range(PERST_2_ACCESS_MIN, PERST_2_ACCESS_MAX);
> @@ -419,11 +1094,29 @@ static const struct dw_pcie_host_ops kirin_pcie_host_ops = {
>         .host_init = kirin_pcie_host_init,
>  };
>
> +struct kirin_pcie_ops kirin960_pcie_ops = {
> +       .get_resource = kirin960_pcie_get_resource,
> +       .power_on = kirin960_pcie_power_on,
> +};
> +
> +struct kirin_pcie_ops kirin970_pcie_ops = {
> +       .get_resource = kirin970_pcie_get_resource,
> +       .power_on = kirin970_pcie_power_on,
> +};
> +
> +static const struct of_device_id kirin_pcie_match[] = {
> +       { .compatible = "hisilicon,kirin960-pcie", .data = &kirin960_pcie_ops },
> +       { .compatible = "hisilicon,kirin970-pcie", .data = &kirin970_pcie_ops },
> +       {},
> +};
> +
>  static int kirin_pcie_probe(struct platform_device *pdev)
>  {
>         struct device *dev = &pdev->dev;
>         struct kirin_pcie *kirin_pcie;
>         struct dw_pcie *pci;
> +       const struct of_device_id *of_id;
> +       struct kirin_pcie_ops *ops;
>         int ret;
>
>         if (!dev->of_node) {
> @@ -431,6 +1124,9 @@ static int kirin_pcie_probe(struct platform_device *pdev)
>                 return -EINVAL;
>         }
>
> +       of_id = of_match_node(kirin_pcie_match, dev->of_node);
> +       ops = (struct kirin_pcie_ops *)of_id->data;

of_device_get_match_data()

> +
>         kirin_pcie = devm_kzalloc(dev, sizeof(struct kirin_pcie), GFP_KERNEL);
>         if (!kirin_pcie)
>                 return -ENOMEM;
> @@ -448,20 +1144,20 @@ static int kirin_pcie_probe(struct platform_device *pdev)
>         if (ret)
>                 return ret;
>
> -       ret = kirin_pcie_get_resource(kirin_pcie, pdev);
> +       ret = ops->get_resource(kirin_pcie, pdev);
>         if (ret)
>                 return ret;
>
> -       kirin_pcie->gpio_id_reset = of_get_named_gpio(dev->of_node,
> +       kirin_pcie->gpio_id_reset[0] = of_get_named_gpio(dev->of_node,
>                                                       "reset-gpios", 0);
> -       if (kirin_pcie->gpio_id_reset == -EPROBE_DEFER) {
> +       if (kirin_pcie->gpio_id_reset[0] == -EPROBE_DEFER) {
>                 return -EPROBE_DEFER;
> -       } else if (!gpio_is_valid(kirin_pcie->gpio_id_reset)) {
> +       } else if (!gpio_is_valid(kirin_pcie->gpio_id_reset[0])) {
>                 dev_err(dev, "unable to get a valid gpio pin\n");
>                 return -ENODEV;
>         }
>
> -       ret = kirin_pcie_power_on(kirin_pcie);
> +       ret = ops->power_on(kirin_pcie);
>         if (ret)
>                 return ret;
>
> @@ -470,11 +1166,6 @@ static int kirin_pcie_probe(struct platform_device *pdev)
>         return dw_pcie_host_init(&pci->pp);
>  }
>
> -static const struct of_device_id kirin_pcie_match[] = {
> -       { .compatible = "hisilicon,kirin960-pcie" },
> -       {},
> -};
> -
>  static struct platform_driver kirin_pcie_driver = {
>         .probe                  = kirin_pcie_probe,
>         .driver                 = {
> --
> 2.29.2
>

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

* Re: [PATCH v2 09/11] PCI: dwc: pcie-kirin: allow using multiple reset GPIOs
  2021-02-03  7:01 ` [PATCH v2 09/11] PCI: dwc: pcie-kirin: allow using multiple reset GPIOs Mauro Carvalho Chehab
  2021-02-03 13:46   ` Mark Brown
@ 2021-02-03 15:18   ` Krzysztof Wilczyński
  2021-02-03 17:50     ` Mauro Carvalho Chehab
  1 sibling, 1 reply; 24+ messages in thread
From: Krzysztof Wilczyński @ 2021-02-03 15:18 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Binghui Wang, Bjorn Helgaas, Liam Girdwood, Lorenzo Pieralisi,
	Mark Brown, Rob Herring, Xiaowei Song, linux-kernel, linux-pci


Hi Mauro,

Thank you for working on this!

> @@ -151,8 +152,10 @@ struct kirin_pcie {
>       struct clk      *phy_ref_clk;
>       struct clk      *pcie_aclk;
>       struct clk      *pcie_aux_clk;
> -     int             gpio_id_reset[4];
> +     int             n_gpio_resets;
>       int             gpio_id_clkreq[3];
> +     int             gpio_id_reset[MAX_GPIO_RESETS];
> +     const char      *reset_names[MAX_GPIO_RESETS];
>       u32             eye_param[5];
>  };
[...]

A small nit, so feel free to ignore, of course.

The "n_gpio_resets" variable might be better as "gpio_resets_num" or
"gpio_resets_count" - both are popular name suffixes for that type of
variables.  To add, other variables also start with "gpio_", thus it
would also follow the naming pattern.

[...]
> +     kirin_pcie->n_gpio_resets = of_gpio_named_count(np, "reset-gpios");
[...]

This would then become (for example):

  kirin_pcie->gpio_resets_count = of_gpio_named_count(np, "reset-gpios");

Krzysztof

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

* Re: [PATCH v2 09/11] PCI: dwc: pcie-kirin: allow using multiple reset GPIOs
  2021-02-03 13:46   ` Mark Brown
@ 2021-02-03 17:28     ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 24+ messages in thread
From: Mauro Carvalho Chehab @ 2021-02-03 17:28 UTC (permalink / raw)
  To: Mark Brown
  Cc: Binghui Wang, Bjorn Helgaas, Liam Girdwood, Lorenzo Pieralisi,
	Rob Herring, Xiaowei Song, linux-kernel, linux-pci

Em Wed, 3 Feb 2021 13:46:20 +0000
Mark Brown <broonie@kernel.org> escreveu:

> On Wed, Feb 03, 2021 at 08:01:53AM +0100, Mauro Carvalho Chehab wrote:
> 
> > +	reg = devm_regulator_get_optional(dev, "pci");
> > +	if (IS_ERR_OR_NULL(reg)) {
> > +		if (PTR_ERR(reg) == -EPROBE_DEFER)
> > +		    return PTR_ERR(reg);
> > +	} else {
> > +		ret = regulator_enable(reg);  
> 
> This is still misuse of regulator_get_optional() for the same reason.

Sorry, it seems to be a badly solved rebase conflict. I'll address it
at the next round.


Thanks,
Mauro

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

* Re: [PATCH v2 04/11] PCI: dwc: pcie-kirin: add support for Kirin 970 PCIe controller
  2021-02-03 14:34   ` Rob Herring
@ 2021-02-03 17:49     ` Mauro Carvalho Chehab
  2021-03-26  8:39     ` Mauro Carvalho Chehab
  2021-07-05 14:54     ` Mauro Carvalho Chehab
  2 siblings, 0 replies; 24+ messages in thread
From: Mauro Carvalho Chehab @ 2021-02-03 17:49 UTC (permalink / raw)
  To: Rob Herring
  Cc: Manivannan Sadhasivam, Binghui Wang, Bjorn Helgaas,
	Lorenzo Pieralisi, Xiaowei Song, linux-kernel, PCI

Em Wed, 3 Feb 2021 08:34:21 -0600
Rob Herring <robh@kernel.org> escreveu:

> On Wed, Feb 3, 2021 at 1:02 AM Mauro Carvalho Chehab
> <mchehab+huawei@kernel.org> wrote:
> >
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> >
> > Add support for HiSilicon Kirin 970 (hi3670) SoC PCIe controller, based
> > on Synopsys DesignWare PCIe controller IP.
> >
> > [mchehab+huawei@kernel.org: fix merge conflicts]
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > ---
> >  drivers/pci/controller/dwc/pcie-kirin.c | 723 +++++++++++++++++++++++-
> >  1 file changed, 707 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
> > index 026fd1e42a55..5925d2b345a8 100644
> > --- a/drivers/pci/controller/dwc/pcie-kirin.c
> > +++ b/drivers/pci/controller/dwc/pcie-kirin.c
> > @@ -29,6 +29,7 @@
> >  #define to_kirin_pcie(x) dev_get_drvdata((x)->dev)
> >
> >  #define REF_CLK_FREQ                   100000000
> > +#define AXI_CLK_FREQ                   207500000
> >
> >  /* PCIe ELBI registers */
> >  #define SOC_PCIECTRL_CTRL0_ADDR                0x000
> > @@ -60,6 +61,65 @@
> >  #define PCIE_DEBOUNCE_PARAM    0xF0F400
> >  #define PCIE_OE_BYPASS         (0x3 << 28)
> >
> > +/* PCIe CTRL registers */
> > +#define SOC_PCIECTRL_CTRL0_ADDR   0x000
> > +#define SOC_PCIECTRL_CTRL1_ADDR   0x004
> > +#define SOC_PCIECTRL_CTRL7_ADDR   0x01c
> > +#define SOC_PCIECTRL_CTRL12_ADDR  0x030
> > +#define SOC_PCIECTRL_CTRL20_ADDR  0x050
> > +#define SOC_PCIECTRL_CTRL21_ADDR  0x054
> > +#define SOC_PCIECTRL_STATE0_ADDR  0x400
> > +
> > +/* PCIe PHY registers */
> > +#define SOC_PCIEPHY_CTRL0_ADDR    0x000
> > +#define SOC_PCIEPHY_CTRL1_ADDR    0x004
> > +#define SOC_PCIEPHY_CTRL2_ADDR    0x008
> > +#define SOC_PCIEPHY_CTRL3_ADDR    0x00c
> > +#define SOC_PCIEPHY_CTRL38_ADDR   0x0098
> > +#define SOC_PCIEPHY_STATE0_ADDR   0x400
> > +
> > +#define PCIE_LINKUP_ENABLE            (0x8020)
> > +#define PCIE_ELBI_SLV_DBI_ENABLE      (0x1 << 21)
> > +#define PCIE_LTSSM_ENABLE_BIT         (0x1 << 11)
> > +#define PCIEPHY_RESET_BIT             (0x1 << 17)
> > +#define PCIEPHY_PIPE_LINE0_RESET_BIT  (0x1 << 19)
> > +
> > +#define PORT_MSI_CTRL_ADDR            0x820
> > +#define PORT_MSI_CTRL_UPPER_ADDR      0x824
> > +#define PORT_MSI_CTRL_INT0_ENABLE     0x828  
> 
> These are common DWC 'port logic' registers. I think the core DWC
> should handle them if not already.

I'll double-check if are there something that can be dropped.

> 
> > +
> > +#define EYEPARAM_NOCFG 0xFFFFFFFF
> > +#define RAWLANEN_DIG_PCS_XF_TX_OVRD_IN_1 0x3001
> > +#define SUP_DIG_LVL_OVRD_IN 0xf
> > +#define LANEN_DIG_ASIC_TX_OVRD_IN_1 0x1002
> > +#define LANEN_DIG_ASIC_TX_OVRD_IN_2 0x1003
> > +
> > +/* kirin970 pciephy register */
> > +#define SOC_PCIEPHY_MMC1PLL_CTRL1  0xc04
> > +#define SOC_PCIEPHY_MMC1PLL_CTRL16 0xC40
> > +#define SOC_PCIEPHY_MMC1PLL_CTRL17 0xC44
> > +#define SOC_PCIEPHY_MMC1PLL_CTRL20 0xC50
> > +#define SOC_PCIEPHY_MMC1PLL_CTRL21 0xC54
> > +#define SOC_PCIEPHY_MMC1PLL_STAT0  0xE00  
> 
> This looks like it is almost all phy related. I think it should all be
> moved to a separate phy driver. Yes, we have some other PCI drivers
> controlling their phys directly where the phy registers are
> intermingled with the PCI host registers, but I think those either
> predate the phy subsystem or are really simple. I also have a dream to
> move all the phy control to the DWC core code.

Yeah, it sounds so. I'll check how hard would be to split this code
into a phy driver on a separate patch.

> > +struct kirin_pcie_ops {
> > +       long (*get_resource)(struct kirin_pcie *kirin_pcie,
> > +                            struct platform_device *pdev);
> > +       int (*power_on)(struct kirin_pcie *kirin_pcie);  
> 
> Never need to power off?

This driver uses devm_*() to allocate its resources, and doesn't
even uses builtin_platform_driver(kirin_pcie_driver) to register.

So, at the current state, it doesn't need a poweroff.

This "power_on" method is actually the device-specific part of
the probe: depending on the compatible, it either runs the Kirin 960
or the new Kirin 970 variant.

I'll try to double-check this when trying to split the PHY logic
from the driver.

> > +static long kirin970_pcie_get_resource(struct kirin_pcie *kirin_pcie,
> > +                                     struct platform_device *pdev)
> > +{
> > +       struct device *dev = &pdev->dev;
> > +       struct resource *apb;
> > +       struct resource *phy;
> > +       struct resource *dbi;
> > +       int ret;
> > +
> > +       apb = platform_get_resource_byname(pdev, IORESOURCE_MEM, "apb");
> > +       kirin_pcie->apb_base = devm_ioremap_resource(dev, apb);
> > +       if (IS_ERR(kirin_pcie->apb_base))
> > +               return PTR_ERR(kirin_pcie->apb_base);
> > +
> > +       phy = platform_get_resource_byname(pdev, IORESOURCE_MEM, "phy");
> > +       kirin_pcie->phy_base = devm_ioremap_resource(dev, phy);
> > +       if (IS_ERR(kirin_pcie->phy_base))
> > +               return PTR_ERR(kirin_pcie->phy_base);
> > +
> > +       dbi = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
> > +       kirin_pcie->pci->dbi_base = devm_ioremap_resource(dev, dbi);  
> 
> The DWC core handles this now.

Yes. This is addressed on this patch:

	[PATCH v2 07/11] PCI: dwc: pcie-kirin: place common init code altogether

> > +       if (IS_ERR(kirin_pcie->pci->dbi_base))
> > +               return PTR_ERR(kirin_pcie->pci->dbi_base);
> > +
> > +       kirin970_pcie_get_eyeparam(kirin_pcie);
> > +
> > +       kirin_pcie->gpio_id_reset[0] = of_get_named_gpio(dev->of_node,
> > +                                               "switch,reset-gpios", 0);
> > +       if (kirin_pcie->gpio_id_reset[0] < 0)
> > +               return -ENODEV;
> > +
> > +       kirin_pcie->gpio_id_reset[1] = of_get_named_gpio(dev->of_node,
> > +                                               "eth,reset-gpios", 0);
> > +       if (kirin_pcie->gpio_id_reset[1] < 0)
> > +               return -ENODEV;
> > +
> > +       kirin_pcie->gpio_id_reset[2] = of_get_named_gpio(dev->of_node,
> > +                                               "m_2,reset-gpios", 0);
> > +       if (kirin_pcie->gpio_id_reset[2] < 0)
> > +               return -ENODEV;
> > +
> > +       kirin_pcie->gpio_id_reset[3] = of_get_named_gpio(dev->of_node,
> > +                                               "mini1,reset-gpios", 0);
> > +       if (kirin_pcie->gpio_id_reset[3] < 0)  
> 
> I've already explained how all this is wrong.

This patch:

	[PATCH v2 09/11] PCI: dwc: pcie-kirin: allow using multiple reset GPIOs

Changes the above to, instead, get all the PERST# reset from "reset-gpios":

	kirin_pcie->n_gpio_resets = of_gpio_named_count(np, "reset-gpios");
	if (kirin_pcie->n_gpio_resets > MAX_GPIO_RESETS) {
		dev_err(dev, "Too many GPIO resets!\n");
		return -EINVAL;
	}
	for (i = 0; i < kirin_pcie->n_gpio_resets; i++) {
		kirin_pcie->gpio_id_reset[i] = of_get_named_gpio(dev->of_node,
							    "reset-gpios", i);
		if (kirin_pcie->gpio_id_reset[i] < 0)
			return kirin_pcie->gpio_id_reset[i];

		sprintf(name, "pcie_perst_%d", i);
		kirin_pcie->reset_names[i] = devm_kstrdup_const(dev, name,
								GFP_KERNEL);
		if (!kirin_pcie->reset_names[i])
			return -ENOMEM;
	}

They're all handled altogether inside the driver.

> > @@ -431,6 +1124,9 @@ static int kirin_pcie_probe(struct platform_device *pdev)
> >                 return -EINVAL;
> >         }
> >
> > +       of_id = of_match_node(kirin_pcie_match, dev->of_node);
> > +       ops = (struct kirin_pcie_ops *)of_id->data;  
> 
> of_device_get_match_data()

Ok.

Thanks,
Mauro

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

* Re: [PATCH v2 09/11] PCI: dwc: pcie-kirin: allow using multiple reset GPIOs
  2021-02-03 15:18   ` Krzysztof Wilczyński
@ 2021-02-03 17:50     ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 24+ messages in thread
From: Mauro Carvalho Chehab @ 2021-02-03 17:50 UTC (permalink / raw)
  To: Krzysztof Wilczyński
  Cc: Binghui Wang, Bjorn Helgaas, Liam Girdwood, Lorenzo Pieralisi,
	Mark Brown, Rob Herring, Xiaowei Song, linux-kernel, linux-pci

Em Wed, 3 Feb 2021 16:18:01 +0100
Krzysztof Wilczyński <kw@linux.com> escreveu:

> Hi Mauro,
> 
> Thank you for working on this!
> 
> > @@ -151,8 +152,10 @@ struct kirin_pcie {
> >       struct clk      *phy_ref_clk;
> >       struct clk      *pcie_aclk;
> >       struct clk      *pcie_aux_clk;
> > -     int             gpio_id_reset[4];
> > +     int             n_gpio_resets;
> >       int             gpio_id_clkreq[3];
> > +     int             gpio_id_reset[MAX_GPIO_RESETS];
> > +     const char      *reset_names[MAX_GPIO_RESETS];
> >       u32             eye_param[5];
> >  };  
> [...]
> 
> A small nit, so feel free to ignore, of course.
> 
> The "n_gpio_resets" variable might be better as "gpio_resets_num" or
> "gpio_resets_count" - both are popular name suffixes for that type of
> variables.  To add, other variables also start with "gpio_", thus it
> would also follow the naming pattern.
> 
> [...]
> > +     kirin_pcie->n_gpio_resets = of_gpio_named_count(np, "reset-gpios");  
> [...]
> 
> This would then become (for example):
> 
>   kirin_pcie->gpio_resets_count = of_gpio_named_count(np, "reset-gpios");

Ok. Will change it at the next round.

Thanks,
Mauro

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

* Re: [PATCH v2 01/11] doc: bindings: PCI: designware-pcie.txt: convert it to YAML
  2021-02-03  7:01 ` [PATCH v2 01/11] doc: bindings: PCI: designware-pcie.txt: convert it to YAML Mauro Carvalho Chehab
@ 2021-02-04 15:20   ` Rob Herring
  0 siblings, 0 replies; 24+ messages in thread
From: Rob Herring @ 2021-02-04 15:20 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Neil Armstrong, Jonathan Hunter, Bjorn Helgaas, NXP Linux Team,
	linux-arm-kernel, Kevin Hilman, linux-kernel, Rob Herring,
	Pengutronix Kernel Team, Kishon Vijay Abraham I,
	Thomas Petazzoni, Shawn Guo, Marek Szyprowski, linux-samsung-soc,
	Jerome Brunet, Jonathan Chocron, devicetree, Jaehoon Chung,
	Sascha Hauer, Thierry Reding, linux-tegra, linux-pci,
	Gustavo Pimentel, Jesper Nilsson, Kunihiko Hayashi,
	Krzysztof Kozlowski, Andy Gross, linux-arm-msm, linux-omap,
	Jingoo Han, linux-amlogic, Zhou Wang, Lucas Stach,
	Martin Blumenstingl, Bjorn Andersson, linux-arm-kernel,
	Fabio Estevam, Richard Zhu

On Wed, 03 Feb 2021 08:01:45 +0100, Mauro Carvalho Chehab wrote:
> Convert the file into a DT schema.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>  .../bindings/pci/amlogic,meson-pcie.txt       |   4 +-
>  .../bindings/pci/axis,artpec6-pcie.txt        |   2 +-
>  .../bindings/pci/designware-pcie.txt          |  77 ----------
>  .../bindings/pci/fsl,imx6q-pcie.txt           |   2 +-
>  .../bindings/pci/hisilicon-histb-pcie.txt     |   2 +-
>  .../bindings/pci/hisilicon-pcie.txt           |   2 +-
>  .../devicetree/bindings/pci/kirin-pcie.txt    |   2 +-
>  .../bindings/pci/layerscape-pci.txt           |   2 +-
>  .../bindings/pci/nvidia,tegra194-pcie.txt     |   4 +-
>  .../devicetree/bindings/pci/pci-armada8k.txt  |   2 +-
>  .../devicetree/bindings/pci/pci-keystone.txt  |  10 +-
>  .../devicetree/bindings/pci/pcie-al.txt       |   2 +-
>  .../devicetree/bindings/pci/qcom,pcie.txt     |  14 +-
>  .../bindings/pci/samsung,exynos-pcie.yaml     |   2 +-
>  .../devicetree/bindings/pci/snps,pcie.yaml    | 139 ++++++++++++++++++
>  .../pci/socionext,uniphier-pcie-ep.yaml       |   2 +-
>  .../devicetree/bindings/pci/ti-pci.txt        |   4 +-
>  .../devicetree/bindings/pci/uniphier-pcie.txt |   2 +-
>  MAINTAINERS                                   |   2 +-
>  19 files changed, 169 insertions(+), 107 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/pci/designware-pcie.txt
>  create mode 100644 Documentation/devicetree/bindings/pci/snps,pcie.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/snps,pcie.yaml: properties:snps,enable-cdm-check: 'oneOf' conditional failed, one must be fixed:
	'type' is a required property
	Additional properties are not allowed ('$ref' was unexpected)
	/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/snps,pcie.yaml: properties:snps,enable-cdm-check: 'oneOf' conditional failed, one must be fixed:
		'enum' is a required property
		'const' is a required property
	'/schemas/types.yaml#definitions/flag' does not match 'types.yaml#/definitions/'
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/snps,pcie.yaml: ignoring, error in schema: properties: snps,enable-cdm-check
warning: no schema found in file: ./Documentation/devicetree/bindings/pci/snps,pcie.yaml

See https://patchwork.ozlabs.org/patch/1435145

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


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

* Re: [PATCH v2 04/11] PCI: dwc: pcie-kirin: add support for Kirin 970 PCIe controller
  2021-02-03 14:34   ` Rob Herring
  2021-02-03 17:49     ` Mauro Carvalho Chehab
@ 2021-03-26  8:39     ` Mauro Carvalho Chehab
  2021-03-26  8:51       ` Manivannan Sadhasivam
  2021-07-05 14:54     ` Mauro Carvalho Chehab
  2 siblings, 1 reply; 24+ messages in thread
From: Mauro Carvalho Chehab @ 2021-03-26  8:39 UTC (permalink / raw)
  To: Rob Herring
  Cc: Manivannan Sadhasivam, Binghui Wang, Bjorn Helgaas,
	Lorenzo Pieralisi, Xiaowei Song, linux-kernel, PCI

Em Wed, 3 Feb 2021 08:34:21 -0600
Rob Herring <robh@kernel.org> escreveu:

> On Wed, Feb 3, 2021 at 1:02 AM Mauro Carvalho Chehab
> <mchehab+huawei@kernel.org> wrote:
> >
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> >
> > Add support for HiSilicon Kirin 970 (hi3670) SoC PCIe controller, based
> > on Synopsys DesignWare PCIe controller IP.
> >
> > [mchehab+huawei@kernel.org: fix merge conflicts]
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > ---
> >  drivers/pci/controller/dwc/pcie-kirin.c | 723 +++++++++++++++++++++++-
> >  1 file changed, 707 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
> > index 026fd1e42a55..5925d2b345a8 100644
> > --- a/drivers/pci/controller/dwc/pcie-kirin.c
> > +++ b/drivers/pci/controller/dwc/pcie-kirin.c
> > @@ -29,6 +29,7 @@
> >  #define to_kirin_pcie(x) dev_get_drvdata((x)->dev)
> >
> >  #define REF_CLK_FREQ                   100000000
> > +#define AXI_CLK_FREQ                   207500000
> >
> >  /* PCIe ELBI registers */
> >  #define SOC_PCIECTRL_CTRL0_ADDR                0x000
> > @@ -60,6 +61,65 @@
> >  #define PCIE_DEBOUNCE_PARAM    0xF0F400
> >  #define PCIE_OE_BYPASS         (0x3 << 28)
> >
> > +/* PCIe CTRL registers */
> > +#define SOC_PCIECTRL_CTRL0_ADDR   0x000
> > +#define SOC_PCIECTRL_CTRL1_ADDR   0x004
> > +#define SOC_PCIECTRL_CTRL7_ADDR   0x01c
> > +#define SOC_PCIECTRL_CTRL12_ADDR  0x030
> > +#define SOC_PCIECTRL_CTRL20_ADDR  0x050
> > +#define SOC_PCIECTRL_CTRL21_ADDR  0x054
> > +#define SOC_PCIECTRL_STATE0_ADDR  0x400
> > +
> > +/* PCIe PHY registers */
> > +#define SOC_PCIEPHY_CTRL0_ADDR    0x000
> > +#define SOC_PCIEPHY_CTRL1_ADDR    0x004
> > +#define SOC_PCIEPHY_CTRL2_ADDR    0x008
> > +#define SOC_PCIEPHY_CTRL3_ADDR    0x00c
> > +#define SOC_PCIEPHY_CTRL38_ADDR   0x0098
> > +#define SOC_PCIEPHY_STATE0_ADDR   0x400
> > +
> > +#define PCIE_LINKUP_ENABLE            (0x8020)
> > +#define PCIE_ELBI_SLV_DBI_ENABLE      (0x1 << 21)
> > +#define PCIE_LTSSM_ENABLE_BIT         (0x1 << 11)
> > +#define PCIEPHY_RESET_BIT             (0x1 << 17)
> > +#define PCIEPHY_PIPE_LINE0_RESET_BIT  (0x1 << 19)
> > +
> > +#define PORT_MSI_CTRL_ADDR            0x820
> > +#define PORT_MSI_CTRL_UPPER_ADDR      0x824
> > +#define PORT_MSI_CTRL_INT0_ENABLE     0x828  
> 
> These are common DWC 'port logic' registers. I think the core DWC
> should handle them if not already.
> 
> > +
> > +#define EYEPARAM_NOCFG 0xFFFFFFFF
> > +#define RAWLANEN_DIG_PCS_XF_TX_OVRD_IN_1 0x3001
> > +#define SUP_DIG_LVL_OVRD_IN 0xf
> > +#define LANEN_DIG_ASIC_TX_OVRD_IN_1 0x1002
> > +#define LANEN_DIG_ASIC_TX_OVRD_IN_2 0x1003
> > +
> > +/* kirin970 pciephy register */
> > +#define SOC_PCIEPHY_MMC1PLL_CTRL1  0xc04
> > +#define SOC_PCIEPHY_MMC1PLL_CTRL16 0xC40
> > +#define SOC_PCIEPHY_MMC1PLL_CTRL17 0xC44
> > +#define SOC_PCIEPHY_MMC1PLL_CTRL20 0xC50
> > +#define SOC_PCIEPHY_MMC1PLL_CTRL21 0xC54
> > +#define SOC_PCIEPHY_MMC1PLL_STAT0  0xE00  
> 
> This looks like it is almost all phy related. I think it should all be
> moved to a separate phy driver. Yes, we have some other PCI drivers
> controlling their phys directly where the phy registers are
> intermingled with the PCI host registers, but I think those either
> predate the phy subsystem or are really simple. I also have a dream to
> move all the phy control to the DWC core code.

Please notice that this patch was not written by me, but, instead,
by Mannivannan. So, I can't change it. What I can certainly do is to
write a separate patch at the end of this series moving the Kirin 970
phy to a separate driver. Would this be accepted?

Btw, what should be done with the Kirin 960 PHY code that it is
already embedded on this driver, and whose some of the DT properties
are for its phy layer? 

Thanks,
Mauro

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

* Re: [PATCH v2 04/11] PCI: dwc: pcie-kirin: add support for Kirin 970 PCIe controller
  2021-03-26  8:39     ` Mauro Carvalho Chehab
@ 2021-03-26  8:51       ` Manivannan Sadhasivam
  2021-03-26 10:04         ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 24+ messages in thread
From: Manivannan Sadhasivam @ 2021-03-26  8:51 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Rob Herring, Binghui Wang, Bjorn Helgaas, Lorenzo Pieralisi,
	Xiaowei Song, linux-kernel, PCI

On Fri, Mar 26, 2021 at 09:39:36AM +0100, Mauro Carvalho Chehab wrote:
> Em Wed, 3 Feb 2021 08:34:21 -0600
> Rob Herring <robh@kernel.org> escreveu:
> 
> > On Wed, Feb 3, 2021 at 1:02 AM Mauro Carvalho Chehab
> > <mchehab+huawei@kernel.org> wrote:
> > >
> > > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > >
> > > Add support for HiSilicon Kirin 970 (hi3670) SoC PCIe controller, based
> > > on Synopsys DesignWare PCIe controller IP.
> > >
> > > [mchehab+huawei@kernel.org: fix merge conflicts]
> > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > > ---
> > >  drivers/pci/controller/dwc/pcie-kirin.c | 723 +++++++++++++++++++++++-
> > >  1 file changed, 707 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
> > > index 026fd1e42a55..5925d2b345a8 100644
> > > --- a/drivers/pci/controller/dwc/pcie-kirin.c
> > > +++ b/drivers/pci/controller/dwc/pcie-kirin.c
> > > @@ -29,6 +29,7 @@
> > 

[...]

> > This looks like it is almost all phy related. I think it should all be
> > moved to a separate phy driver. Yes, we have some other PCI drivers
> > controlling their phys directly where the phy registers are
> > intermingled with the PCI host registers, but I think those either
> > predate the phy subsystem or are really simple. I also have a dream to
> > move all the phy control to the DWC core code.
> 
> Please notice that this patch was not written by me, but, instead,
> by Mannivannan. So, I can't change it.

Feel free to move the PHY pieces to a separate PHY driver as suggested.
My driver code was merely WIP one and I don't have any objection to
change the patch.

I'd be happy if you add my Co-developed tag to the PCIe driver patch with
the SoB ofc.

> What I can certainly do is to
> write a separate patch at the end of this series moving the Kirin 970
> phy to a separate driver. Would this be accepted?
> 

Ah, please don't do that. I know that you've already followed the same
process for other HiSi drivers but that looks messy IMO.

> Btw, what should be done with the Kirin 960 PHY code that it is
> already embedded on this driver, and whose some of the DT properties
> are for its phy layer? 
> 

You might need to create a PHY driver for both 960 and 970. I don't see
any harm there. But please make sure you test the patches on both boards.

Thanks,
Mani

> Thanks,
> Mauro

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

* Re: [PATCH v2 04/11] PCI: dwc: pcie-kirin: add support for Kirin 970 PCIe controller
  2021-03-26  8:51       ` Manivannan Sadhasivam
@ 2021-03-26 10:04         ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 24+ messages in thread
From: Mauro Carvalho Chehab @ 2021-03-26 10:04 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Rob Herring, Binghui Wang, Bjorn Helgaas, Lorenzo Pieralisi,
	Xiaowei Song, linux-kernel, PCI

Em Fri, 26 Mar 2021 14:21:02 +0530
Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> escreveu:

> On Fri, Mar 26, 2021 at 09:39:36AM +0100, Mauro Carvalho Chehab wrote:
> > Em Wed, 3 Feb 2021 08:34:21 -0600
> > Rob Herring <robh@kernel.org> escreveu:
> >   
> > > On Wed, Feb 3, 2021 at 1:02 AM Mauro Carvalho Chehab
> > > <mchehab+huawei@kernel.org> wrote:  
> > > >
> > > > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > >
> > > > Add support for HiSilicon Kirin 970 (hi3670) SoC PCIe controller, based
> > > > on Synopsys DesignWare PCIe controller IP.
> > > >
> > > > [mchehab+huawei@kernel.org: fix merge conflicts]
> > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > > > ---
> > > >  drivers/pci/controller/dwc/pcie-kirin.c | 723 +++++++++++++++++++++++-
> > > >  1 file changed, 707 insertions(+), 16 deletions(-)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
> > > > index 026fd1e42a55..5925d2b345a8 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-kirin.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-kirin.c
> > > > @@ -29,6 +29,7 @@  
> > >   
> 
> [...]
> 
> > > This looks like it is almost all phy related. I think it should all be
> > > moved to a separate phy driver. Yes, we have some other PCI drivers
> > > controlling their phys directly where the phy registers are
> > > intermingled with the PCI host registers, but I think those either
> > > predate the phy subsystem or are really simple. I also have a dream to
> > > move all the phy control to the DWC core code.  
> > 
> > Please notice that this patch was not written by me, but, instead,
> > by Mannivannan. So, I can't change it.  
> 
> Feel free to move the PHY pieces to a separate PHY driver as suggested.
> My driver code was merely WIP one and I don't have any objection to
> change the patch.
> 
> I'd be happy if you add my Co-developed tag to the PCIe driver patch with
> the SoB ofc.

Ok.

> > What I can certainly do is to
> > write a separate patch at the end of this series moving the Kirin 970
> > phy to a separate driver. Would this be accepted?
> >   
> 
> Ah, please don't do that. I know that you've already followed the same
> process for other HiSi drivers but that looks messy IMO.

The problem is related to licensing issues and US export regulations. 

By preserving the patches from the original authors, I played safe.

> 
> > Btw, what should be done with the Kirin 960 PHY code that it is
> > already embedded on this driver, and whose some of the DT properties
> > are for its phy layer? 
> >   
> 
> You might need to create a PHY driver for both 960 and 970. I don't see
> any harm there. But please make sure you test the patches on both boards.

Testing on Kirin 960 will be harder. Well, I can get my hands on one
such board, but right now I don't have any M.2 device I can spare with,
in order to test.

This will also break DT backward compatibility, as, for instance, the
PERST# gpio seems to be part of the Kirin 960 PHY, as Kirin 970 has
different PERST# logic: on Kirin 970, there's one PERST# GPIO per PCIe
device.

So, before starting such change, I need to know if DT maintainers
are OK with that.

Thanks,
Mauro

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

* Re: [PATCH v2 04/11] PCI: dwc: pcie-kirin: add support for Kirin 970 PCIe controller
  2021-02-03 14:34   ` Rob Herring
  2021-02-03 17:49     ` Mauro Carvalho Chehab
  2021-03-26  8:39     ` Mauro Carvalho Chehab
@ 2021-07-05 14:54     ` Mauro Carvalho Chehab
  2 siblings, 0 replies; 24+ messages in thread
From: Mauro Carvalho Chehab @ 2021-07-05 14:54 UTC (permalink / raw)
  To: Rob Herring
  Cc: Manivannan Sadhasivam, Binghui Wang, Bjorn Helgaas,
	Lorenzo Pieralisi, Xiaowei Song, linux-kernel, PCI

Hi Rob,

Em Wed, 3 Feb 2021 08:34:21 -0600
Rob Herring <robh@kernel.org> escreveu:

> On Wed, Feb 3, 2021 at 1:02 AM Mauro Carvalho Chehab
> <mchehab+huawei@kernel.org> wrote:
> >
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> >
> > Add support for HiSilicon Kirin 970 (hi3670) SoC PCIe controller, based
> > on Synopsys DesignWare PCIe controller IP.
> >

> > +/* kirin970 pciephy register */
> > +#define SOC_PCIEPHY_MMC1PLL_CTRL1  0xc04
> > +#define SOC_PCIEPHY_MMC1PLL_CTRL16 0xC40
> > +#define SOC_PCIEPHY_MMC1PLL_CTRL17 0xC44
> > +#define SOC_PCIEPHY_MMC1PLL_CTRL20 0xC50
> > +#define SOC_PCIEPHY_MMC1PLL_CTRL21 0xC54
> > +#define SOC_PCIEPHY_MMC1PLL_STAT0  0xE00  
> 
> This looks like it is almost all phy related. I think it should all be
> moved to a separate phy driver. Yes, we have some other PCI drivers
> controlling their phys directly where the phy registers are
> intermingled with the PCI host registers, but I think those either
> predate the phy subsystem or are really simple. I also have a dream to
> move all the phy control to the DWC core code.

I'm looking into it right now, but it sounds that splitting the PHY
part of the driver can't be done on a DT backward-compatible way.

See, the current pcie-kirin has already a PHY driver for Kirin 960
embedded on it. So, before adding PHY support to the driver, we need
to split the current driver on two, placing the PHY part of pcie-kirin.c
into a drivers/phy/hisilicon/phy-hi3660-pcie.c driver, as the current
code contain lots of PHY code already on it. See, for instance the priv
driver struct:

	struct kirin_pcie {
		struct dw_pcie	*pci;
		void __iomem	*apb_base;
		void __iomem	*phy_base;
		struct regmap	*crgctrl;
		struct regmap	*sysctrl;
		struct clk	*apb_sys_clk;
		struct clk	*apb_phy_clk;
		struct clk	*phy_ref_clk;
		struct clk	*pcie_aclk;
		struct clk	*pcie_aux_clk;
		int		gpio_id_reset;
	};

There are at three PHY-related fields there:

		- an iomem region: phy_base
		- two clocks: phy_ref_clk and apb_phy_clk

Yet, right now, they're all declared under this DT property at
arch/arm64/boot/dts/hisilicon/hi3660.dtsi:

	pcie@f4000000 {
                compatible = "hisilicon,kirin960-pcie";
		reg = <0x0 0xf4000000 0x0 0x1000>,
                      <0x0 0xff3fe000 0x0 0x1000>,
                      <0x0 0xf3f20000 0x0 0x40000>,
                      <0x0 0xf5000000 0x0 0x2000>;
                reg-names = "dbi", "apb", "phy", "config";
	...
		clocks = <&crg_ctrl HI3660_PCIEPHY_REF>,
                         <&crg_ctrl HI3660_CLK_GATE_PCIEAUX>,
                         <&crg_ctrl HI3660_PCLK_GATE_PCIE_PHY>,
                         <&crg_ctrl HI3660_PCLK_GATE_PCIE_SYS>,
                         <&crg_ctrl HI3660_ACLK_GATE_PCIE>;
                clock-names = "pcie_phy_ref", "pcie_aux",
                          "pcie_apb_phy", "pcie_apb_sys",
                          "pcie_aclk";
	...
	};

If we split the PHY out of this driver, the above would be, instead:

	pcie_phy: pcie-phy@f3f2000 {
		compatible = "";
		reg = <0xf3f200000 0x40000>;

		clocks = <&crg_ctrl HI3660_PCIEPHY_REF>,
                         <&crg_ctrl HI3660_PCLK_GATE_PCIE_PHY>,
                clock-names = "pcie_phy_ref", "pcie_apb_phy";
	}

	pcie@f4000000 {
                compatible = "hisilicon,kirin960-pcie";
		reg = <0x0 0xf4000000 0x0 0x1000>,
                      <0x0 0xff3fe000 0x0 0x1000>,
                      <0x0 0xf5000000 0x0 0x2000>;
                reg-names = "dbi", "apb", "config";
	...
		clocks = <&crg_ctrl HI3660_CLK_GATE_PCIEAUX>,
                         <&crg_ctrl HI3660_PCLK_GATE_PCIE_SYS>,
                         <&crg_ctrl HI3660_ACLK_GATE_PCIE>;
                clock-names = "pcie_aux", "pcie_apb_sys",
                              "pcie_aclk";

		phys = <&pcie_phy>;
	...
	};

Would a change like that be accepted? If not, IMO the best would be to
also merge the Hikey 970 PHY inside the same driver, in order to avoid
DT regressions.

Thanks,
Mauro

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

end of thread, other threads:[~2021-07-05 14:54 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-03  7:01 [PATCH v2 00/11] Add support for Hikey 970 PCIe Mauro Carvalho Chehab
2021-02-03  7:01 ` [PATCH v2 01/11] doc: bindings: PCI: designware-pcie.txt: convert it to YAML Mauro Carvalho Chehab
2021-02-04 15:20   ` Rob Herring
2021-02-03  7:01 ` [PATCH v2 02/11] doc: bindings: kirin-pcie.txt: " Mauro Carvalho Chehab
2021-02-03  7:01 ` [PATCH v2 03/11] doc: bindings: add new parameters used by Kirin 970 Mauro Carvalho Chehab
2021-02-03  7:01 ` [PATCH v2 04/11] PCI: dwc: pcie-kirin: add support for Kirin 970 PCIe controller Mauro Carvalho Chehab
2021-02-03  9:32   ` kernel test robot
2021-02-03 14:34   ` Rob Herring
2021-02-03 17:49     ` Mauro Carvalho Chehab
2021-03-26  8:39     ` Mauro Carvalho Chehab
2021-03-26  8:51       ` Manivannan Sadhasivam
2021-03-26 10:04         ` Mauro Carvalho Chehab
2021-07-05 14:54     ` Mauro Carvalho Chehab
2021-02-03  7:01 ` [PATCH v2 05/11] PCI: dwc: pcie-kirin: simplify error handling logic Mauro Carvalho Chehab
2021-02-03  7:01 ` [PATCH v2 06/11] PCI: dwc: pcie-kirin: simplify Kirin 970 get resource logic Mauro Carvalho Chehab
2021-02-03  7:01 ` [PATCH v2 07/11] PCI: dwc: pcie-kirin: place common init code altogether Mauro Carvalho Chehab
2021-02-03  7:01 ` [PATCH v2 08/11] PCI: dwc: pcie-kirin: add support for a regulator Mauro Carvalho Chehab
2021-02-03  7:01 ` [PATCH v2 09/11] PCI: dwc: pcie-kirin: allow using multiple reset GPIOs Mauro Carvalho Chehab
2021-02-03 13:46   ` Mark Brown
2021-02-03 17:28     ` Mauro Carvalho Chehab
2021-02-03 15:18   ` Krzysztof Wilczyński
2021-02-03 17:50     ` Mauro Carvalho Chehab
2021-02-03  7:01 ` [PATCH v2 10/11] PCI: dwc: pcie-kirin: add support for clkreq GPIOs Mauro Carvalho Chehab
2021-02-03  7:01 ` [PATCH v2 11/11] pci: dwc: pcie-kirin: cleanup kirin970_pcie_get_eyeparam() Mauro Carvalho Chehab

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