[PATCH v7 00/10] Add support for Hikey 970 PCIe

[PATCH v7 00/10] Add support for Hikey 970 PCIe

[Mauro Carvalho Chehab]

This series depends on this one:
	https://lore.kernel.org/lkml/cover.1626515862.git.mchehab+huawei@kernel.org/

It is available, with its patch dependencies against v5.14-rc1 at:
	https://github.com/mchehab/linux/commits/pcie-alternate

This series add support at the pcie-kirin dirver for it to use a separate PHY driver.

Yet, in order to preserve the existing DT schema for Kirin 960, it keeps the
Kirin 960 PHY inside the pci driver. I tried to find a way to split it while keeping
the DT schema backward-compatible, but currently the PHY core doesn't allow
that, as it relies on a "phy" property at the pcie node in order to recognize the
PHY driver.

Once the pci-kiring is modified to support an external PHY driver, add a
Kirin 970 PHY and add the needed properties for the HiKey 970 board to
detect the PCIe.

It should be noticed that the HiKey 970 design uses 4 different GPIO pins, one 
for each PERST# signal for each PCIe bus device:

    - GPIO 56 has a pullup logic from 1V8 to 2V5
      connected to a PCIe bridge chip (PEX 8606);
    - GPIO 25 has a pullup logic from 1V8 to 3V3
      connected to the PERST# pin at the M.2 slot;
    - GPIO 220 has a pullup logic from 1V8 to 3V3
      connected to the PERST# pin at the PCIe mini slot;
    - GPIO 203 has a pullup logic from 1V8 to 3V3
      connected to the PERST# pin at the Ethernet chipset.

At the first versions, those were mapped as part of the pci-bus, but the
pci-bus.yaml schema only allows a single PERST# GPIO. So, on v5, those
were moved to the PHY DT schema. However, as Rob complained, on this
version, I opted to add a separate patch (the last one) that moves those
back to the PCIe of-node. 

If such patch 09/09 is accepted, then this patch for the DT schema should 
also be accepted:

   https://github.com/devicetree-org/dt-schema/pull/56

Tested on Hikey970:

  $ lspci
  00:00.0 PCI bridge: Huawei Technologies Co., Ltd. Device 3670 (rev 01)
  01:00.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
  02:01.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
  02:04.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
  02:05.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
  02:07.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
  02:09.0 PCI bridge: PLX Technology, Inc. PEX 8606 6 Lane, 6 Port PCI Express Gen 2 (5.0 GT/s) Switch (rev ba)
  06:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 07)

  $ ethtool enp6s0
  Settings for enp6s0:
	Supported ports: [ TP	 MII ]
	Supported link modes:   10baseT/Half 10baseT/Full
	                        100baseT/Half 100baseT/Full
	                        1000baseT/Half 1000baseT/Full
	Supported pause frame use: Symmetric Receive-only
	Supports auto-negotiation: Yes
	Supported FEC modes: Not reported
	Advertised link modes:  10baseT/Half 10baseT/Full
	                        100baseT/Half 100baseT/Full
	                        1000baseT/Half 1000baseT/Full
	Advertised pause frame use: Symmetric Receive-only
	Advertised auto-negotiation: Yes
	Advertised FEC modes: Not reported
	Link partner advertised link modes:  10baseT/Half 10baseT/Full
	                                     100baseT/Half 100baseT/Full
	Link partner advertised pause frame use: Symmetric Receive-only
	Link partner advertised auto-negotiation: Yes
	Link partner advertised FEC modes: Not reported
	Speed: 100Mb/s
	Duplex: Full
	Auto-negotiation: on
	master-slave cfg: preferred slave
	master-slave status: slave
	Port: Twisted Pair
	PHYAD: 0
	Transceiver: external
	MDI-X: Unknown
  netlink error: Operation not permitted
	Link detected: yes

Also tested  that Hikey 960 keeps being supported:

  $ lspci
  00:00.0 PCI bridge: Huawei Technologies Co., Ltd. Device 3660 (rev 01)

---

v7:
- Moved kirin_pcie_match to be closer to the probe function;
- Improved patch description for:
	"PCI: kirin: add support for a PHY layer"
- Added missing MODULE_*() macros on both PCI and PHY drivers;
- Fixed a warning at hisilicon,phy-hi3670-pcie.yaml reported by
  Rob Herring's bot.

v6:
- Use an alternative approach, in order to keep the Kirin 960 PHY internal to 
  the driver, in order to not break the DT schema. The PHY-specific code
  were made self-contained at pcie-kirin, in order to make easier to split
  it in the future, if needed.

v5:
- added "static" to hi3670_pcie_get_eyeparam() declaration on patch 6/8

v4:

- dropped the DTS patch, as it depends on a PMIC-related patch series;
- minor changes at the patch description;
- HiKey and HiSilicon are now using the preferred CamelCase format.


Manivannan Sadhasivam (1):
  arm64: dts: HiSilicon: Add support for HiKey 970 PCIe controller
    hardware

Mauro Carvalho Chehab (9):
  PCI: kirin: Reorganize the PHY logic inside the driver
  PCI: kirin: Add support for a PHY layer
  PCI: kirin: Use regmap for APB registers
  PCI: kirin: Add MODULE_* macros
  dt-bindings: PCI: kirin: Fix compatible string
  dt-bindings: phy: Add bindings for HiKey 970 PCIe PHY
  phy: HiSilicon: Add driver for Kirin 970 PCIe PHY
  dt-bindings: PCI: kirin-pcie.txt: Convert it to yaml
  phy-hi3670-pcie: Move reset-gpios to the PCIe DT schema

 .../bindings/pci/hisilicon,kirin-pcie.yaml    |  87 ++
 .../devicetree/bindings/pci/kirin-pcie.txt    |  50 -
 .../devicetree/bindings/pci/snps,dw-pcie.yaml |   2 +-
 .../phy/hisilicon,phy-hi3670-pcie.yaml        |  88 ++
 MAINTAINERS                                   |   2 +-
 arch/arm64/boot/dts/hisilicon/hi3670.dtsi     |  70 ++
 .../boot/dts/hisilicon/hikey970-pmic.dtsi     |   1 -
 drivers/pci/controller/dwc/pcie-kirin.c       | 413 +++++---
 drivers/phy/hisilicon/Kconfig                 |  10 +
 drivers/phy/hisilicon/Makefile                |   1 +
 drivers/phy/hisilicon/phy-hi3670-pcie.c       | 902 ++++++++++++++++++
 11 files changed, 1429 insertions(+), 197 deletions(-)
 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/phy/hisilicon,phy-hi3670-pcie.yaml
 create mode 100644 drivers/phy/hisilicon/phy-hi3670-pcie.c

-- 
2.31.1

[PATCH v7 01/10] PCI: kirin: Reorganize the PHY logic inside the driver

[Mauro Carvalho Chehab]

The pcie-kirin PCIe driver contains internally a PHY interface for
Kirin 960.

As the next patches will add support for using an external PHY
driver, reorganize the driver in a way that the PHY part
will be self-contained.

This could be moved to a separate PHY driver, but a change
like that would mean a non-backward-compatible DT schema
change.

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

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index 026fd1e42a55..b4063a3434df 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -28,26 +28,16 @@
 
 #define to_kirin_pcie(x) dev_get_drvdata((x)->dev)
 
-#define REF_CLK_FREQ			100000000
-
 /* PCIe ELBI registers */
 #define SOC_PCIECTRL_CTRL0_ADDR		0x000
 #define SOC_PCIECTRL_CTRL1_ADDR		0x004
-#define SOC_PCIEPHY_CTRL2_ADDR		0x008
-#define SOC_PCIEPHY_CTRL3_ADDR		0x00c
 #define PCIE_ELBI_SLV_DBI_ENABLE	(0x1 << 21)
 
 /* info located in APB */
 #define PCIE_APP_LTSSM_ENABLE	0x01c
-#define PCIE_APB_PHY_CTRL0	0x0
-#define PCIE_APB_PHY_CTRL1	0x4
 #define PCIE_APB_PHY_STATUS0	0x400
 #define PCIE_LINKUP_ENABLE	(0x8020)
 #define PCIE_LTSSM_ENABLE_BIT	(0x1 << 11)
-#define PIPE_CLK_STABLE		(0x1 << 19)
-#define PHY_REF_PAD_BIT		(0x1 << 8)
-#define PHY_PWR_DOWN_BIT	(0x1 << 22)
-#define PHY_RST_ACK_BIT		(0x1 << 16)
 
 /* info located in sysctrl */
 #define SCTRL_PCIE_CMOS_OFFSET	0x60
@@ -60,6 +50,29 @@
 #define PCIE_DEBOUNCE_PARAM	0xF0F400
 #define PCIE_OE_BYPASS		(0x3 << 28)
 
+struct kirin_pcie {
+	struct dw_pcie	*pci;
+	struct phy	*phy;
+	void __iomem	*apb_base;
+	void		*phy_priv;	/* Needed for Kirin 960 PHY */
+};
+
+/*
+ * Kirin 960 PHY. Can't be split into a PHY driver without changing the
+ * DT schema.
+ */
+
+#define REF_CLK_FREQ			100000000
+
+/* PHY info located in APB */
+#define PCIE_APB_PHY_CTRL0	0x0
+#define PCIE_APB_PHY_CTRL1	0x4
+#define PCIE_APB_PHY_STATUS0   0x400
+#define PIPE_CLK_STABLE		BIT(19)
+#define PHY_REF_PAD_BIT		BIT(8)
+#define PHY_PWR_DOWN_BIT	BIT(22)
+#define PHY_RST_ACK_BIT		BIT(16)
+
 /* peri_crg ctrl */
 #define CRGCTRL_PCIE_ASSERT_OFFSET	0x88
 #define CRGCTRL_PCIE_ASSERT_BIT		0x8c000000
@@ -69,8 +82,6 @@
 #define REF_2_PERST_MAX		25000
 #define PERST_2_ACCESS_MIN	10000
 #define PERST_2_ACCESS_MAX	12000
-#define LINK_WAIT_MIN		900
-#define LINK_WAIT_MAX		1000
 #define PIPE_CLK_WAIT_MIN	550
 #define PIPE_CLK_WAIT_MAX	600
 #define TIME_CMOS_MIN		100
@@ -78,118 +89,112 @@
 #define TIME_PHY_PD_MIN		10
 #define TIME_PHY_PD_MAX		11
 
-struct kirin_pcie {
-	struct dw_pcie	*pci;
-	void __iomem	*apb_base;
-	void __iomem	*phy_base;
+struct hi3660_pcie_phy {
+	struct device	*dev;
+	void __iomem	*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;
+	struct clk	*aclk;
+	struct clk	*aux_clk;
 	int		gpio_id_reset;
 };
 
-/* Registers in PCIeCTRL */
-static inline void kirin_apb_ctrl_writel(struct kirin_pcie *kirin_pcie,
-					 u32 val, u32 reg)
-{
-	writel(val, kirin_pcie->apb_base + reg);
-}
-
-static inline u32 kirin_apb_ctrl_readl(struct kirin_pcie *kirin_pcie, u32 reg)
-{
-	return readl(kirin_pcie->apb_base + reg);
-}
-
 /* Registers in PCIePHY */
-static inline void kirin_apb_phy_writel(struct kirin_pcie *kirin_pcie,
+static inline void kirin_apb_phy_writel(struct hi3660_pcie_phy *hi3660_pcie_phy,
 					u32 val, u32 reg)
 {
-	writel(val, kirin_pcie->phy_base + reg);
+	writel(val, hi3660_pcie_phy->base + reg);
 }
 
-static inline u32 kirin_apb_phy_readl(struct kirin_pcie *kirin_pcie, u32 reg)
+static inline u32 kirin_apb_phy_readl(struct hi3660_pcie_phy *hi3660_pcie_phy,
+				      u32 reg)
 {
-	return readl(kirin_pcie->phy_base + reg);
+	return readl(hi3660_pcie_phy->base + reg);
 }
 
-static long kirin_pcie_get_clk(struct kirin_pcie *kirin_pcie,
-			       struct platform_device *pdev)
+static int hi3660_pcie_phy_get_clk(struct hi3660_pcie_phy *phy)
 {
-	struct device *dev = &pdev->dev;
+	struct device *dev = phy->dev;
 
-	kirin_pcie->phy_ref_clk = devm_clk_get(dev, "pcie_phy_ref");
-	if (IS_ERR(kirin_pcie->phy_ref_clk))
-		return PTR_ERR(kirin_pcie->phy_ref_clk);
+	phy->phy_ref_clk = devm_clk_get(dev, "pcie_phy_ref");
+	if (IS_ERR(phy->phy_ref_clk))
+		return PTR_ERR(phy->phy_ref_clk);
 
-	kirin_pcie->pcie_aux_clk = devm_clk_get(dev, "pcie_aux");
-	if (IS_ERR(kirin_pcie->pcie_aux_clk))
-		return PTR_ERR(kirin_pcie->pcie_aux_clk);
+	phy->aux_clk = devm_clk_get(dev, "pcie_aux");
+	if (IS_ERR(phy->aux_clk))
+		return PTR_ERR(phy->aux_clk);
 
-	kirin_pcie->apb_phy_clk = devm_clk_get(dev, "pcie_apb_phy");
-	if (IS_ERR(kirin_pcie->apb_phy_clk))
-		return PTR_ERR(kirin_pcie->apb_phy_clk);
+	phy->apb_phy_clk = devm_clk_get(dev, "pcie_apb_phy");
+	if (IS_ERR(phy->apb_phy_clk))
+		return PTR_ERR(phy->apb_phy_clk);
 
-	kirin_pcie->apb_sys_clk = devm_clk_get(dev, "pcie_apb_sys");
-	if (IS_ERR(kirin_pcie->apb_sys_clk))
-		return PTR_ERR(kirin_pcie->apb_sys_clk);
+	phy->apb_sys_clk = devm_clk_get(dev, "pcie_apb_sys");
+	if (IS_ERR(phy->apb_sys_clk))
+		return PTR_ERR(phy->apb_sys_clk);
 
-	kirin_pcie->pcie_aclk = devm_clk_get(dev, "pcie_aclk");
-	if (IS_ERR(kirin_pcie->pcie_aclk))
-		return PTR_ERR(kirin_pcie->pcie_aclk);
+	phy->aclk = devm_clk_get(dev, "pcie_aclk");
+	if (IS_ERR(phy->aclk))
+		return PTR_ERR(phy->aclk);
 
 	return 0;
 }
 
-static long kirin_pcie_get_resource(struct kirin_pcie *kirin_pcie,
-				    struct platform_device *pdev)
+static int hi3660_pcie_phy_get_resource(struct hi3660_pcie_phy *phy)
 {
-	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))
-		return PTR_ERR(kirin_pcie->crgctrl);
-
-	kirin_pcie->sysctrl =
-		syscon_regmap_lookup_by_compatible("hisilicon,hi3660-sctrl");
-	if (IS_ERR(kirin_pcie->sysctrl))
-		return PTR_ERR(kirin_pcie->sysctrl);
+	struct device *dev = phy->dev;
+	struct platform_device *pdev;
+
+	/* registers */
+	pdev = container_of(dev, struct platform_device, dev);
+
+	phy->base = devm_platform_ioremap_resource_byname(pdev, "phy");
+	if (IS_ERR(phy->base))
+		return PTR_ERR(phy->base);
+
+	phy->crgctrl = syscon_regmap_lookup_by_compatible("hisilicon,hi3660-crgctrl");
+	if (IS_ERR(phy->crgctrl))
+		return PTR_ERR(phy->crgctrl);
+
+	phy->sysctrl = syscon_regmap_lookup_by_compatible("hisilicon,hi3660-sctrl");
+	if (IS_ERR(phy->sysctrl))
+		return PTR_ERR(phy->sysctrl);
+
+	/* gpios */
+	phy->gpio_id_reset = of_get_named_gpio(dev->of_node,
+					       "reset-gpios", 0);
+	if (phy->gpio_id_reset == -EPROBE_DEFER) {
+		return -EPROBE_DEFER;
+	} else if (!gpio_is_valid(phy->gpio_id_reset)) {
+		dev_err(phy->dev, "unable to get a valid gpio pin\n");
+		return -ENODEV;
+	}
 
 	return 0;
 }
 
-static int kirin_pcie_phy_init(struct kirin_pcie *kirin_pcie)
+static int hi3660_pcie_phy_start(struct hi3660_pcie_phy *phy)
 {
-	struct device *dev = kirin_pcie->pci->dev;
+	struct device *dev = phy->dev;
 	u32 reg_val;
 
-	reg_val = kirin_apb_phy_readl(kirin_pcie, PCIE_APB_PHY_CTRL1);
+	reg_val = kirin_apb_phy_readl(phy, PCIE_APB_PHY_CTRL1);
 	reg_val &= ~PHY_REF_PAD_BIT;
-	kirin_apb_phy_writel(kirin_pcie, reg_val, PCIE_APB_PHY_CTRL1);
+	kirin_apb_phy_writel(phy, reg_val, PCIE_APB_PHY_CTRL1);
 
-	reg_val = kirin_apb_phy_readl(kirin_pcie, PCIE_APB_PHY_CTRL0);
+	reg_val = kirin_apb_phy_readl(phy, PCIE_APB_PHY_CTRL0);
 	reg_val &= ~PHY_PWR_DOWN_BIT;
-	kirin_apb_phy_writel(kirin_pcie, reg_val, PCIE_APB_PHY_CTRL0);
+	kirin_apb_phy_writel(phy, reg_val, PCIE_APB_PHY_CTRL0);
 	usleep_range(TIME_PHY_PD_MIN, TIME_PHY_PD_MAX);
 
-	reg_val = kirin_apb_phy_readl(kirin_pcie, PCIE_APB_PHY_CTRL1);
+	reg_val = kirin_apb_phy_readl(phy, PCIE_APB_PHY_CTRL1);
 	reg_val &= ~PHY_RST_ACK_BIT;
-	kirin_apb_phy_writel(kirin_pcie, reg_val, PCIE_APB_PHY_CTRL1);
+	kirin_apb_phy_writel(phy, reg_val, PCIE_APB_PHY_CTRL1);
 
 	usleep_range(PIPE_CLK_WAIT_MIN, PIPE_CLK_WAIT_MAX);
-	reg_val = kirin_apb_phy_readl(kirin_pcie, PCIE_APB_PHY_STATUS0);
+	reg_val = kirin_apb_phy_readl(phy, PCIE_APB_PHY_STATUS0);
 	if (reg_val & PIPE_CLK_STABLE) {
 		dev_err(dev, "PIPE clk is not stable\n");
 		return -EINVAL;
@@ -198,105 +203,157 @@ static int kirin_pcie_phy_init(struct kirin_pcie *kirin_pcie)
 	return 0;
 }
 
-static void kirin_pcie_oe_enable(struct kirin_pcie *kirin_pcie)
+static void hi3660_pcie_phy_oe_enable(struct hi3660_pcie_phy *phy)
 {
 	u32 val;
 
-	regmap_read(kirin_pcie->sysctrl, SCTRL_PCIE_OE_OFFSET, &val);
+	regmap_read(phy->sysctrl, SCTRL_PCIE_OE_OFFSET, &val);
 	val |= PCIE_DEBOUNCE_PARAM;
 	val &= ~PCIE_OE_BYPASS;
-	regmap_write(kirin_pcie->sysctrl, SCTRL_PCIE_OE_OFFSET, val);
+	regmap_write(phy->sysctrl, SCTRL_PCIE_OE_OFFSET, val);
 }
 
-static int kirin_pcie_clk_ctrl(struct kirin_pcie *kirin_pcie, bool enable)
+static int hi3660_pcie_phy_clk_ctrl(struct hi3660_pcie_phy *phy, bool enable)
 {
 	int ret = 0;
 
 	if (!enable)
 		goto close_clk;
 
-	ret = clk_set_rate(kirin_pcie->phy_ref_clk, REF_CLK_FREQ);
+	ret = clk_set_rate(phy->phy_ref_clk, REF_CLK_FREQ);
 	if (ret)
 		return ret;
 
-	ret = clk_prepare_enable(kirin_pcie->phy_ref_clk);
+	ret = clk_prepare_enable(phy->phy_ref_clk);
 	if (ret)
 		return ret;
 
-	ret = clk_prepare_enable(kirin_pcie->apb_sys_clk);
+	ret = clk_prepare_enable(phy->apb_sys_clk);
 	if (ret)
 		goto apb_sys_fail;
 
-	ret = clk_prepare_enable(kirin_pcie->apb_phy_clk);
+	ret = clk_prepare_enable(phy->apb_phy_clk);
 	if (ret)
 		goto apb_phy_fail;
 
-	ret = clk_prepare_enable(kirin_pcie->pcie_aclk);
+	ret = clk_prepare_enable(phy->aclk);
 	if (ret)
 		goto aclk_fail;
 
-	ret = clk_prepare_enable(kirin_pcie->pcie_aux_clk);
+	ret = clk_prepare_enable(phy->aux_clk);
 	if (ret)
 		goto aux_clk_fail;
 
 	return 0;
 
 close_clk:
-	clk_disable_unprepare(kirin_pcie->pcie_aux_clk);
+	clk_disable_unprepare(phy->aux_clk);
 aux_clk_fail:
-	clk_disable_unprepare(kirin_pcie->pcie_aclk);
+	clk_disable_unprepare(phy->aclk);
 aclk_fail:
-	clk_disable_unprepare(kirin_pcie->apb_phy_clk);
+	clk_disable_unprepare(phy->apb_phy_clk);
 apb_phy_fail:
-	clk_disable_unprepare(kirin_pcie->apb_sys_clk);
+	clk_disable_unprepare(phy->apb_sys_clk);
 apb_sys_fail:
-	clk_disable_unprepare(kirin_pcie->phy_ref_clk);
+	clk_disable_unprepare(phy->phy_ref_clk);
 
 	return ret;
 }
 
-static int kirin_pcie_power_on(struct kirin_pcie *kirin_pcie)
+static int hi3660_pcie_phy_power_on(struct kirin_pcie *pcie)
 {
+	struct hi3660_pcie_phy *phy = pcie->phy_priv;
 	int ret;
 
 	/* Power supply for Host */
-	regmap_write(kirin_pcie->sysctrl,
+	regmap_write(phy->sysctrl,
 		     SCTRL_PCIE_CMOS_OFFSET, SCTRL_PCIE_CMOS_BIT);
 	usleep_range(TIME_CMOS_MIN, TIME_CMOS_MAX);
-	kirin_pcie_oe_enable(kirin_pcie);
 
-	ret = kirin_pcie_clk_ctrl(kirin_pcie, true);
+	hi3660_pcie_phy_oe_enable(phy);
+
+	ret = hi3660_pcie_phy_clk_ctrl(phy, true);
 	if (ret)
 		return ret;
 
 	/* ISO disable, PCIeCtrl, PHY assert and clk gate clear */
-	regmap_write(kirin_pcie->sysctrl,
+	regmap_write(phy->sysctrl,
 		     SCTRL_PCIE_ISO_OFFSET, SCTRL_PCIE_ISO_BIT);
-	regmap_write(kirin_pcie->crgctrl,
+	regmap_write(phy->crgctrl,
 		     CRGCTRL_PCIE_ASSERT_OFFSET, CRGCTRL_PCIE_ASSERT_BIT);
-	regmap_write(kirin_pcie->sysctrl,
+	regmap_write(phy->sysctrl,
 		     SCTRL_PCIE_HPCLK_OFFSET, SCTRL_PCIE_HPCLK_BIT);
 
-	ret = kirin_pcie_phy_init(kirin_pcie);
+	ret = hi3660_pcie_phy_start(phy);
 	if (ret)
-		goto close_clk;
+		goto disable_clks;
 
 	/* perst assert Endpoint */
-	if (!gpio_request(kirin_pcie->gpio_id_reset, "pcie_perst")) {
+	if (!gpio_request(phy->gpio_id_reset, "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(phy->gpio_id_reset, 1);
 		if (ret)
-			goto close_clk;
+			goto disable_clks;
 		usleep_range(PERST_2_ACCESS_MIN, PERST_2_ACCESS_MAX);
-
 		return 0;
 	}
 
-close_clk:
-	kirin_pcie_clk_ctrl(kirin_pcie, false);
+disable_clks:
+	hi3660_pcie_phy_clk_ctrl(phy, false);
 	return ret;
 }
 
+static int hi3660_pcie_phy_init(struct platform_device *pdev,
+				struct kirin_pcie *pcie)
+{
+	struct device *dev = &pdev->dev;
+	struct hi3660_pcie_phy *phy;
+	int ret;
+
+	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
+	if (!phy)
+		return -ENOMEM;
+
+	pcie->phy_priv = phy;
+	phy->dev = dev;
+
+	/* registers */
+	pdev = container_of(dev, struct platform_device, dev);
+
+	ret = hi3660_pcie_phy_get_clk(phy);
+	if (ret)
+		return ret;
+
+	return hi3660_pcie_phy_get_resource(phy);
+}
+
+/*
+ * The non-PHY part starts here
+ */
+
+/* Registers in PCIeCTRL */
+static inline void kirin_apb_ctrl_writel(struct kirin_pcie *kirin_pcie,
+					 u32 val, u32 reg)
+{
+	writel(val, kirin_pcie->apb_base + reg);
+}
+
+static inline u32 kirin_apb_ctrl_readl(struct kirin_pcie *kirin_pcie, u32 reg)
+{
+	return readl(kirin_pcie->apb_base + reg);
+}
+
+static long kirin_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);
+
+	return 0;
+}
+
 static void kirin_pcie_sideband_dbi_w_mode(struct kirin_pcie *kirin_pcie,
 					   bool on)
 {
@@ -444,7 +501,7 @@ static int kirin_pcie_probe(struct platform_device *pdev)
 	pci->pp.ops = &kirin_pcie_host_ops;
 	kirin_pcie->pci = pci;
 
-	ret = kirin_pcie_get_clk(kirin_pcie, pdev);
+	ret = hi3660_pcie_phy_init(pdev, kirin_pcie);
 	if (ret)
 		return ret;
 
@@ -452,16 +509,7 @@ static int kirin_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	kirin_pcie->gpio_id_reset = of_get_named_gpio(dev->of_node,
-						      "reset-gpios", 0);
-	if (kirin_pcie->gpio_id_reset == -EPROBE_DEFER) {
-		return -EPROBE_DEFER;
-	} else if (!gpio_is_valid(kirin_pcie->gpio_id_reset)) {
-		dev_err(dev, "unable to get a valid gpio pin\n");
-		return -ENODEV;
-	}
-
-	ret = kirin_pcie_power_on(kirin_pcie);
+	ret = hi3660_pcie_phy_power_on(kirin_pcie);
 	if (ret)
 		return ret;
 
@@ -479,8 +527,8 @@ static struct platform_driver kirin_pcie_driver = {
 	.probe			= kirin_pcie_probe,
 	.driver			= {
 		.name			= "kirin-pcie",
-		.of_match_table = kirin_pcie_match,
-		.suppress_bind_attrs = true,
+		.of_match_table		= kirin_pcie_match,
+		.suppress_bind_attrs	= true,
 	},
 };
 builtin_platform_driver(kirin_pcie_driver);
-- 
2.31.1

[PATCH v7 02/10] PCI: kirin: Add support for a PHY layer

[Mauro Carvalho Chehab]

The pcie-kirin driver contains both PHY and generic PCI driver
on it.

The best would be, instead, to support a PCI PHY driver, making
the driver more generic.

However, it is too late to remove the Kirin 960 PHY, as a change
like that would make the DT schema incompatible with past versions.

So, add support for an external PHY driver without removing the
existing Kirin 960 PHY from it.

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

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index b4063a3434df..31514a5d4bb4 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -8,16 +8,18 @@
  * Author: Xiaowei Song <songxiaowei@huawei.com>
  */
 
-#include <linux/compiler.h>
 #include <linux/clk.h>
+#include <linux/compiler.h>
 #include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/gpio.h>
 #include <linux/interrupt.h>
 #include <linux/mfd/syscon.h>
 #include <linux/of_address.h>
+#include <linux/of_device.h>
 #include <linux/of_gpio.h>
 #include <linux/of_pci.h>
+#include <linux/phy/phy.h>
 #include <linux/pci.h>
 #include <linux/pci_regs.h>
 #include <linux/platform_device.h>
@@ -50,11 +52,18 @@
 #define PCIE_DEBOUNCE_PARAM	0xF0F400
 #define PCIE_OE_BYPASS		(0x3 << 28)
 
+enum pcie_kirin_phy_type {
+	PCIE_KIRIN_INTERNAL_PHY,
+	PCIE_KIRIN_EXTERNAL_PHY
+};
+
 struct kirin_pcie {
+	enum pcie_kirin_phy_type	type;
+
 	struct dw_pcie	*pci;
 	struct phy	*phy;
 	void __iomem	*apb_base;
-	void		*phy_priv;	/* Needed for Kirin 960 PHY */
+	void		*phy_priv;	/* only for PCIE_KIRIN_INTERNAL_PHY */
 };
 
 /*
@@ -476,8 +485,63 @@ static const struct dw_pcie_host_ops kirin_pcie_host_ops = {
 	.host_init = kirin_pcie_host_init,
 };
 
+static int kirin_pcie_power_on(struct platform_device *pdev,
+			       struct kirin_pcie *kirin_pcie)
+{
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	if (kirin_pcie->type == PCIE_KIRIN_INTERNAL_PHY) {
+		ret = hi3660_pcie_phy_init(pdev, kirin_pcie);
+		if (ret)
+			return ret;
+
+		return hi3660_pcie_phy_power_on(kirin_pcie);
+	}
+
+	kirin_pcie->phy = devm_of_phy_get(dev, dev->of_node, NULL);
+	if (IS_ERR(kirin_pcie->phy))
+		return PTR_ERR(kirin_pcie->phy);
+
+	ret = phy_init(kirin_pcie->phy);
+	if (ret)
+		goto err;
+
+	ret = phy_power_on(kirin_pcie->phy);
+	if (ret)
+		goto err;
+
+	return 0;
+err:
+	phy_exit(kirin_pcie->phy);
+	return ret;
+}
+
+static int __exit kirin_pcie_remove(struct platform_device *pdev)
+{
+	struct kirin_pcie *kirin_pcie = platform_get_drvdata(pdev);
+
+	if (kirin_pcie->type == PCIE_KIRIN_INTERNAL_PHY)
+		return 0;
+
+	phy_power_off(kirin_pcie->phy);
+	phy_exit(kirin_pcie->phy);
+
+	return 0;
+}
+
+static const struct of_device_id kirin_pcie_match[] = {
+	{
+		.compatible = "hisilicon,kirin960-pcie",
+		.data = (void *)PCIE_KIRIN_INTERNAL_PHY
+	},
+	{},
+};
+
 static int kirin_pcie_probe(struct platform_device *pdev)
 {
+	enum pcie_kirin_phy_type phy_type;
+	const struct of_device_id *of_id;
 	struct device *dev = &pdev->dev;
 	struct kirin_pcie *kirin_pcie;
 	struct dw_pcie *pci;
@@ -488,6 +552,14 @@ static int kirin_pcie_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	of_id = of_match_device(kirin_pcie_match, dev);
+	if (!of_id) {
+		dev_err(dev, "OF data missing\n");
+		return -EINVAL;
+	}
+
+	phy_type = (enum pcie_kirin_phy_type)of_id->data;
+
 	kirin_pcie = devm_kzalloc(dev, sizeof(struct kirin_pcie), GFP_KERNEL);
 	if (!kirin_pcie)
 		return -ENOMEM;
@@ -500,31 +572,24 @@ static int kirin_pcie_probe(struct platform_device *pdev)
 	pci->ops = &kirin_dw_pcie_ops;
 	pci->pp.ops = &kirin_pcie_host_ops;
 	kirin_pcie->pci = pci;
-
-	ret = hi3660_pcie_phy_init(pdev, kirin_pcie);
-	if (ret)
-		return ret;
+	kirin_pcie->type = phy_type;
 
 	ret = kirin_pcie_get_resource(kirin_pcie, pdev);
 	if (ret)
 		return ret;
 
-	ret = hi3660_pcie_phy_power_on(kirin_pcie);
-	if (ret)
-		return ret;
-
 	platform_set_drvdata(pdev, kirin_pcie);
 
+	ret = kirin_pcie_power_on(pdev, kirin_pcie);
+	if (ret)
+		return ret;
+
 	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,
+	.remove	        	= __exit_p(kirin_pcie_remove),
 	.driver			= {
 		.name			= "kirin-pcie",
 		.of_match_table		= kirin_pcie_match,
-- 
2.31.1

[PATCH v7 03/10] PCI: kirin: Use regmap for APB registers

[Mauro Carvalho Chehab]

The PHY layer need to access APB registers too, for Kirin 970.
So, place them into a named regmap.

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

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index 31514a5d4bb4..5fe0cd0af572 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -61,8 +61,8 @@ struct kirin_pcie {
 	enum pcie_kirin_phy_type	type;
 
 	struct dw_pcie	*pci;
+	struct regmap   *apb;
 	struct phy	*phy;
-	void __iomem	*apb_base;
 	void		*phy_priv;	/* only for PCIE_KIRIN_INTERNAL_PHY */
 };
 
@@ -340,6 +340,13 @@ static int hi3660_pcie_phy_init(struct platform_device *pdev,
  * The non-PHY part starts here
  */
 
+static const struct regmap_config pcie_kirin_regmap_conf = {
+	.name = "kirin_pcie_apb",
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+};
+
 /* Registers in PCIeCTRL */
 static inline void kirin_apb_ctrl_writel(struct kirin_pcie *kirin_pcie,
 					 u32 val, u32 reg)
@@ -355,10 +362,17 @@ static inline u32 kirin_apb_ctrl_readl(struct kirin_pcie *kirin_pcie, u32 reg)
 static long kirin_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);
+	struct device *dev = &pdev->dev;
+	void __iomem *apb_base;
+
+	apb_base = devm_platform_ioremap_resource_byname(pdev, "apb");
+	if (IS_ERR(apb_base))
+		return PTR_ERR(apb_base);
+
+	kirin_pcie->apb = devm_regmap_init_mmio(dev, apb_base,
+						&pcie_kirin_regmap_conf);
+	if (IS_ERR(kirin_pcie->apb))
+		return PTR_ERR(kirin_pcie->apb);
 
 	return 0;
 }
@@ -368,13 +382,13 @@ static void kirin_pcie_sideband_dbi_w_mode(struct kirin_pcie *kirin_pcie,
 {
 	u32 val;
 
-	val = kirin_apb_ctrl_readl(kirin_pcie, SOC_PCIECTRL_CTRL0_ADDR);
+	regmap_read(kirin_pcie->apb, SOC_PCIECTRL_CTRL0_ADDR, &val);
 	if (on)
 		val = val | PCIE_ELBI_SLV_DBI_ENABLE;
 	else
 		val = val & ~PCIE_ELBI_SLV_DBI_ENABLE;
 
-	kirin_apb_ctrl_writel(kirin_pcie, val, SOC_PCIECTRL_CTRL0_ADDR);
+	regmap_write(kirin_pcie->apb, SOC_PCIECTRL_CTRL0_ADDR, val);
 }
 
 static void kirin_pcie_sideband_dbi_r_mode(struct kirin_pcie *kirin_pcie,
@@ -382,13 +396,13 @@ static void kirin_pcie_sideband_dbi_r_mode(struct kirin_pcie *kirin_pcie,
 {
 	u32 val;
 
-	val = kirin_apb_ctrl_readl(kirin_pcie, SOC_PCIECTRL_CTRL1_ADDR);
+	regmap_read(kirin_pcie->apb, SOC_PCIECTRL_CTRL1_ADDR, &val);
 	if (on)
 		val = val | PCIE_ELBI_SLV_DBI_ENABLE;
 	else
 		val = val & ~PCIE_ELBI_SLV_DBI_ENABLE;
 
-	kirin_apb_ctrl_writel(kirin_pcie, val, SOC_PCIECTRL_CTRL1_ADDR);
+	regmap_write(kirin_pcie->apb, SOC_PCIECTRL_CTRL1_ADDR, val);
 }
 
 static int kirin_pcie_rd_own_conf(struct pci_bus *bus, unsigned int devfn,
@@ -448,8 +462,9 @@ static void kirin_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base,
 static int kirin_pcie_link_up(struct dw_pcie *pci)
 {
 	struct kirin_pcie *kirin_pcie = to_kirin_pcie(pci);
-	u32 val = kirin_apb_ctrl_readl(kirin_pcie, PCIE_APB_PHY_STATUS0);
+	u32 val;
 
+	regmap_read(kirin_pcie->apb, PCIE_APB_PHY_STATUS0, &val);
 	if ((val & PCIE_LINKUP_ENABLE) == PCIE_LINKUP_ENABLE)
 		return 1;
 
@@ -461,8 +476,8 @@ static int kirin_pcie_start_link(struct dw_pcie *pci)
 	struct kirin_pcie *kirin_pcie = to_kirin_pcie(pci);
 
 	/* assert LTSSM enable */
-	kirin_apb_ctrl_writel(kirin_pcie, PCIE_LTSSM_ENABLE_BIT,
-			      PCIE_APP_LTSSM_ENABLE);
+	regmap_write(kirin_pcie->apb, PCIE_APP_LTSSM_ENABLE,
+		     PCIE_LTSSM_ENABLE_BIT);
 
 	return 0;
 }
-- 
2.31.1

[PATCH v7 04/10] PCI: kirin: Add MODULE_* macros

[Mauro Carvalho Chehab]

This driver misses the MODULE_* macros. Add them.

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

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index 5fe0cd0af572..a32f7f36461c 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -612,3 +612,8 @@ static struct platform_driver kirin_pcie_driver = {
 	},
 };
 builtin_platform_driver(kirin_pcie_driver);
+
+MODULE_DEVICE_TABLE(of, kirin_pcie_match);
+MODULE_DESCRIPTION("PCIe host controller driver for Kirin Phone SoCs");
+MODULE_AUTHOR("Xiaowei Song <songxiaowei@huawei.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.31.1

[PATCH v7 05/10] dt-bindings: PCI: kirin: Fix compatible string

[Mauro Carvalho Chehab]

The pcie-kirin driver doesn't declare a hisilicon,kirin-pcie.
Also, remove the useless comment after the description, as other
compat will be supported by the same driver in the future.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 Documentation/devicetree/bindings/pci/kirin-pcie.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/kirin-pcie.txt b/Documentation/devicetree/bindings/pci/kirin-pcie.txt
index 7db30534498f..7adab8999a6a 100644
--- a/Documentation/devicetree/bindings/pci/kirin-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/kirin-pcie.txt
@@ -9,7 +9,7 @@ Additional properties are described here:
 
 Required properties
 - compatible:
-	"hisilicon,kirin960-pcie" for PCIe of Kirin960 SoC
+	"hisilicon,kirin960-pcie"
 - reg: Should contain rc_dbi, apb, phy, config registers location and length.
 - reg-names: Must include the following entries:
   "dbi": controller configuration registers;
@@ -23,7 +23,7 @@ Optional properties:
 Example based on kirin960:
 
 	pcie@f4000000 {
-		compatible = "hisilicon,kirin-pcie";
+		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";
-- 
2.31.1

[PATCH v7 07/10] phy: HiSilicon: Add driver for Kirin 970 PCIe PHY

[Mauro Carvalho Chehab]

The Kirin 970 PHY is somewhat similar to the Kirin 960, but it
does a lot more. Add the needed bits for PCIe to start working on
HiKey 970 boards.

Co-developed-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/pci/controller/dwc/pcie-kirin.c |   4 +
 drivers/phy/hisilicon/Kconfig           |  10 +
 drivers/phy/hisilicon/Makefile          |   1 +
 drivers/phy/hisilicon/phy-hi3670-pcie.c | 898 ++++++++++++++++++++++++
 4 files changed, 913 insertions(+)
 create mode 100644 drivers/phy/hisilicon/phy-hi3670-pcie.c

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index a32f7f36461c..bfc0513f7b15 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -550,6 +550,10 @@ static const struct of_device_id kirin_pcie_match[] = {
 		.compatible = "hisilicon,kirin960-pcie",
 		.data = (void *)PCIE_KIRIN_INTERNAL_PHY
 	},
+	{
+		.compatible = "hisilicon,kirin970-pcie",
+		.data = (void *)PCIE_KIRIN_EXTERNAL_PHY
+	},
 	{},
 };
 
diff --git a/drivers/phy/hisilicon/Kconfig b/drivers/phy/hisilicon/Kconfig
index 4d008cfc279c..d3b92c288554 100644
--- a/drivers/phy/hisilicon/Kconfig
+++ b/drivers/phy/hisilicon/Kconfig
@@ -33,6 +33,16 @@ config PHY_HI3670_USB
 
 	  To compile this driver as a module, choose M here.
 
+config PHY_HI3670_PCIE
+	tristate "hi3670 PCIe PHY support"
+	depends on (ARCH_HISI && ARM64) || COMPILE_TEST
+	select GENERIC_PHY
+	select MFD_SYSCON
+	help
+	  Enable this to support the HiSilicon hi3670 PCIe PHY.
+
+	  To compile this driver as a module, choose M here.
+
 config PHY_HISTB_COMBPHY
 	tristate "HiSilicon STB SoCs COMBPHY support"
 	depends on (ARCH_HISI && ARM64) || COMPILE_TEST
diff --git a/drivers/phy/hisilicon/Makefile b/drivers/phy/hisilicon/Makefile
index 51729868145b..4029d3813b1e 100644
--- a/drivers/phy/hisilicon/Makefile
+++ b/drivers/phy/hisilicon/Makefile
@@ -2,6 +2,7 @@
 obj-$(CONFIG_PHY_HI6220_USB)		+= phy-hi6220-usb.o
 obj-$(CONFIG_PHY_HI3660_USB)		+= phy-hi3660-usb3.o
 obj-$(CONFIG_PHY_HI3670_USB)		+= phy-hi3670-usb3.o
+obj-$(CONFIG_PHY_HI3670_PCIE)		+= phy-hi3670-pcie.o
 obj-$(CONFIG_PHY_HISTB_COMBPHY)		+= phy-histb-combphy.o
 obj-$(CONFIG_PHY_HISI_INNO_USB2)	+= phy-hisi-inno-usb2.o
 obj-$(CONFIG_PHY_HIX5HD2_SATA)		+= phy-hix5hd2-sata.o
diff --git a/drivers/phy/hisilicon/phy-hi3670-pcie.c b/drivers/phy/hisilicon/phy-hi3670-pcie.c
new file mode 100644
index 000000000000..0bc0203e9140
--- /dev/null
+++ b/drivers/phy/hisilicon/phy-hi3670-pcie.c
@@ -0,0 +1,898 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PCIe phy driver for Kirin 970
+ *
+ * Copyright (C) 2017 HiSilicon Electronics Co., Ltd.
+ *		https://www.huawei.com
+ * Copyright (C) 2021 Huawei Technologies Co., Ltd.
+ *		https://www.huawei.com
+ *
+ * Authors:
+ *	Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
+ *	Manivannan Sadhasivam <mani@kernel.org>
+ *
+ * Based on:
+ * 	https://lore.kernel.org/lkml/4c9d6581478aa966698758c0420933f5defab4dd.1612335031.git.mchehab+huawei@kernel.org/
+ */
+
+#include <linux/clk.h>
+#include <linux/gpio.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_gpio.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define AXI_CLK_FREQ			207500000
+#define REF_CLK_FREQ			100000000
+
+/* 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
+
+/* hi3670 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
+
+/* Number of GPIOs required by PHY */
+#define MAX_GPIO_RESETS		4
+#define MAX_GPIO_CLKREQ		3
+#define NUM_EYEPARAM		5
+
+/* info located in sysctrl */
+#define SCTRL_PCIE_CMOS_OFFSET	0x60
+#define SCTRL_PCIE_CMOS_BIT	0x10
+#define SCTRL_PCIE_ISO_OFFSET	0x44
+#define SCTRL_PCIE_ISO_BIT	0x30
+#define SCTRL_PCIE_HPCLK_OFFSET	0x190
+#define SCTRL_PCIE_HPCLK_BIT	0x184000
+#define SCTRL_PCIE_OE_OFFSET	0x14a
+#define PCIE_DEBOUNCE_PARAM	0xF0F400
+#define PCIE_OE_BYPASS		(0x3 << 28)
+
+/* peri_crg ctrl */
+#define CRGCTRL_PCIE_ASSERT_OFFSET	0x88
+#define CRGCTRL_PCIE_ASSERT_BIT		0x8c000000
+
+/* Time for delay */
+#define REF_2_PERST_MIN		20000
+#define REF_2_PERST_MAX		25000
+#define PERST_2_ACCESS_MIN	10000
+#define PERST_2_ACCESS_MAX	12000
+#define PIPE_CLK_WAIT_MIN	550
+#define PIPE_CLK_WAIT_MAX	600
+#define TIME_CMOS_MIN		100
+#define TIME_CMOS_MAX		105
+#define TIME_PHY_PD_MIN		10
+#define TIME_PHY_PD_MAX		11
+
+struct hi3670_pcie_phy {
+	struct device	*dev;
+	void __iomem	*base;
+	struct regmap	*apb;
+	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	*aclk;
+	struct clk	*aux_clk;
+	int		n_gpio_resets;
+	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[NUM_EYEPARAM];
+};
+
+
+/* Registers in PCIePHY */
+static inline void hi3670_apb_phy_writel(struct hi3670_pcie_phy *phy,
+					 u32 val, u32 reg)
+{
+	writel(val, phy->base + 0x40000 + reg);
+}
+
+static inline u32 hi3670_apb_phy_readl(struct hi3670_pcie_phy *phy, u32 reg)
+{
+	return readl(phy->base + 0x40000 + reg);
+}
+
+static inline void kirin_apb_natural_phy_writel(struct hi3670_pcie_phy *phy,
+						u32 val, u32 reg)
+{
+	writel(val, phy->base + reg * 4);
+}
+
+static inline u32 kirin_apb_natural_phy_readl(struct hi3670_pcie_phy *phy,
+					      u32 reg)
+{
+	return readl(phy->base + reg * 4);
+}
+
+static void hi3670_pcie_phy_oe_enable(struct hi3670_pcie_phy *phy)
+{
+	u32 val;
+
+	regmap_read(phy->sysctrl, SCTRL_PCIE_OE_OFFSET, &val);
+	val |= PCIE_DEBOUNCE_PARAM;
+	val &= ~PCIE_OE_BYPASS;
+	regmap_write(phy->sysctrl, SCTRL_PCIE_OE_OFFSET, val);
+}
+
+static void hi3670_pcie_get_eyeparam(struct hi3670_pcie_phy *phy)
+{
+	struct device *dev = phy->dev;
+	struct device_node *np;
+	int ret, i;
+
+	np = dev->of_node;
+
+	ret = of_property_read_u32_array(np, "hisilicon,eye-diagram-param",
+					 phy->eye_param, NUM_EYEPARAM);
+	if (!ret)
+		return;
+
+	/* There's no optional eye_param property. Set array to default */
+	for (i = 0; i < NUM_EYEPARAM; i++)
+		phy->eye_param[i] = EYEPARAM_NOCFG;
+}
+
+static void hi3670_pcie_set_eyeparam(struct hi3670_pcie_phy *phy)
+{
+	u32 val;
+
+	val = kirin_apb_natural_phy_readl(phy, RAWLANEN_DIG_PCS_XF_TX_OVRD_IN_1);
+
+	if (phy->eye_param[1] != EYEPARAM_NOCFG) {
+		val &= (~0xf00);
+		val |= (phy->eye_param[1] << 8) | (0x1 << 12);
+	}
+	kirin_apb_natural_phy_writel(phy, val, RAWLANEN_DIG_PCS_XF_TX_OVRD_IN_1);
+
+	val = kirin_apb_natural_phy_readl(phy, LANEN_DIG_ASIC_TX_OVRD_IN_2);
+	val &= (~0x1FBF);
+	if (phy->eye_param[2] != EYEPARAM_NOCFG)
+		val |= (phy->eye_param[2]<< 0) | (0x1 << 6);
+
+	if (phy->eye_param[3] != EYEPARAM_NOCFG)
+		val |= (phy->eye_param[3] << 7) | (0x1 << 13);
+
+	kirin_apb_natural_phy_writel(phy, val, LANEN_DIG_ASIC_TX_OVRD_IN_2);
+
+	val = kirin_apb_natural_phy_readl(phy, SUP_DIG_LVL_OVRD_IN);
+	if (phy->eye_param[0] != EYEPARAM_NOCFG) {
+		val &= (~0x1C0);
+		val |= (phy->eye_param[0] << 6) | (0x1 << 9);
+	}
+	kirin_apb_natural_phy_writel(phy, val, SUP_DIG_LVL_OVRD_IN);
+
+	val = kirin_apb_natural_phy_readl(phy, LANEN_DIG_ASIC_TX_OVRD_IN_1);
+	if (phy->eye_param[4] != EYEPARAM_NOCFG) {
+		val &= (~0x7E00);
+		val |= (phy->eye_param[4] << 9) | (0x1 << 15);
+	}
+	kirin_apb_natural_phy_writel(phy, val, LANEN_DIG_ASIC_TX_OVRD_IN_1);
+}
+
+static int hi3670_pcie_gpio_request(struct hi3670_pcie_phy *phy,
+				    struct device *dev)
+{
+	int ret, i;
+
+	for (i = 0; i < phy->n_gpio_resets; i++) {
+		if (!gpio_is_valid(phy->gpio_id_reset[i])) {
+			dev_err(dev, "unable to get a valid %s gpio\n",
+				phy->reset_names[i]);
+			return -ENODEV;
+		}
+
+		ret = devm_gpio_request(dev, phy->gpio_id_reset[i],
+					phy->reset_names[i]);
+		if (ret)
+			return ret;
+	}
+
+	for (i = 0; i < phy->n_gpio_clkreq; i++) {
+		if (!gpio_is_valid(phy->gpio_id_clkreq[i])) {
+			dev_err(dev, "unable to get a valid %s gpio\n",
+				phy->clkreq_names[i]);
+			return -ENODEV;
+		}
+
+		ret = devm_gpio_request(dev, phy->gpio_id_clkreq[i],
+					phy->clkreq_names[i]);
+		if (ret)
+			return ret;
+	}
+
+	return ret;
+}
+
+static void hi3670_pcie_natural_cfg(struct hi3670_pcie_phy *phy)
+{
+	u32 val;
+
+	/* change 2p mem_ctrl */
+	regmap_write(phy->apb, SOC_PCIECTRL_CTRL20_ADDR, 0x02605550);
+
+	/* pull up sys_aux_pwr_det */
+	regmap_read(phy->apb, SOC_PCIECTRL_CTRL7_ADDR, &val);
+	val |= (0x1 << 10);
+	regmap_write(phy->apb, SOC_PCIECTRL_CTRL7_ADDR, val);
+
+	/* output, pull down */
+	regmap_read(phy->apb, SOC_PCIECTRL_CTRL12_ADDR, &val);
+	val &= ~(0x3 << 2);
+	val |= (0x1 << 1);
+	val &= ~(0x1 << 0);
+	regmap_write(phy->apb, SOC_PCIECTRL_CTRL12_ADDR, val);
+
+	/* Handle phy_reset and lane0_reset to HW */
+	val = hi3670_apb_phy_readl(phy, SOC_PCIEPHY_CTRL1_ADDR);
+	val |= PCIEPHY_RESET_BIT;
+	val &= ~PCIEPHY_PIPE_LINE0_RESET_BIT;
+	hi3670_apb_phy_writel(phy, val, SOC_PCIEPHY_CTRL1_ADDR);
+
+	/* fix chip bug: TxDetectRx fail */
+	val = hi3670_apb_phy_readl(phy, SOC_PCIEPHY_CTRL38_ADDR);
+	val |= (0x1 << 2);
+	hi3670_apb_phy_writel(phy, val, SOC_PCIEPHY_CTRL38_ADDR);
+}
+
+static void hi3670_pcie_pll_init(struct hi3670_pcie_phy *phy)
+{
+	u32 val;
+
+	/* choose FNPLL */
+	val = hi3670_apb_phy_readl(phy, SOC_PCIEPHY_MMC1PLL_CTRL1);
+	val |= (0x1 << 27);
+	hi3670_apb_phy_writel(phy, val, SOC_PCIEPHY_MMC1PLL_CTRL1);
+
+	val = hi3670_apb_phy_readl(phy, SOC_PCIEPHY_MMC1PLL_CTRL16);
+	val &= 0xF000FFFF;
+	/* fnpll fbdiv = 0xD0 */
+	val |= (0xd0 << 16);
+	hi3670_apb_phy_writel(phy, val, SOC_PCIEPHY_MMC1PLL_CTRL16);
+
+	val = hi3670_apb_phy_readl(phy, SOC_PCIEPHY_MMC1PLL_CTRL17);
+	val &= 0xFF000000;
+	/* fnpll fracdiv = 0x555555 */
+	val |= (0x555555 << 0);
+	hi3670_apb_phy_writel(phy, val, SOC_PCIEPHY_MMC1PLL_CTRL17);
+
+	val = hi3670_apb_phy_readl(phy, 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);
+	hi3670_apb_phy_writel(phy, val, SOC_PCIEPHY_MMC1PLL_CTRL20);
+
+	hi3670_apb_phy_writel(phy, 0x20, SOC_PCIEPHY_MMC1PLL_CTRL21);
+}
+
+static int hi3670_pcie_pll_ctrl(struct hi3670_pcie_phy *phy, bool enable)
+{
+	struct device *dev = phy->dev;
+	u32 val;
+	int time = 200;
+
+	if (enable) {
+		/* pd = 0 */
+		val = hi3670_apb_phy_readl(phy, SOC_PCIEPHY_MMC1PLL_CTRL16);
+		val &= ~(0x1 << 0);
+		hi3670_apb_phy_writel(phy, val, SOC_PCIEPHY_MMC1PLL_CTRL16);
+
+		val = hi3670_apb_phy_readl(phy, 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 = hi3670_apb_phy_readl(phy, SOC_PCIEPHY_MMC1PLL_STAT0);
+		}
+
+		/* pciepll_bp = 0 */
+		val = hi3670_apb_phy_readl(phy, SOC_PCIEPHY_MMC1PLL_CTRL20);
+		val &= ~(0x1 << 16);
+		hi3670_apb_phy_writel(phy, val, SOC_PCIEPHY_MMC1PLL_CTRL20);
+
+	} else {
+		/* pd = 1 */
+		val = hi3670_apb_phy_readl(phy, SOC_PCIEPHY_MMC1PLL_CTRL16);
+		val |= (0x1 << 0);
+		hi3670_apb_phy_writel(phy, val, SOC_PCIEPHY_MMC1PLL_CTRL16);
+
+		/* pciepll_bp = 1 */
+		val = hi3670_apb_phy_readl(phy, SOC_PCIEPHY_MMC1PLL_CTRL20);
+		val |= (0x1 << 16);
+		hi3670_apb_phy_writel(phy, val, SOC_PCIEPHY_MMC1PLL_CTRL20);
+	}
+
+	 return 0;
+}
+
+static void hi3670_pcie_hp_debounce_gt(struct hi3670_pcie_phy *phy, bool open)
+{
+	if (open)
+		/* gt_clk_pcie_hp/gt_clk_pcie_debounce open */
+		regmap_write(phy->crgctrl, CRGPERIPH_PEREN12, 0x9000);
+	else
+		/* gt_clk_pcie_hp/gt_clk_pcie_debounce close */
+		regmap_write(phy->crgctrl, CRGPERIPH_PERDIS12, 0x9000);
+}
+
+static void hi3670_pcie_phyref_gt(struct hi3670_pcie_phy *phy, bool open)
+{
+	unsigned int val;
+
+	regmap_read(phy->crgctrl, CRGPERIPH_PCIECTRL0, &val);
+
+	if (open)
+		val &= ~(0x1 << 1); //enable hard gt mode
+	else
+		val |= (0x1 << 1); //disable hard gt mode
+
+	regmap_write(phy->crgctrl, CRGPERIPH_PCIECTRL0, val);
+
+	/* disable soft gt mode */
+	regmap_write(phy->crgctrl, CRGPERIPH_PERDIS12, 0x4000);
+}
+
+static void hi3670_pcie_oe_ctrl(struct hi3670_pcie_phy *phy, bool en_flag)
+{
+	unsigned int val;
+
+	regmap_read(phy->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(phy->crgctrl, CRGPERIPH_PCIECTRL0, val);
+}
+
+static void hi3670_pcie_ioref_gt(struct hi3670_pcie_phy *phy, bool open)
+{
+	unsigned int val;
+
+	if (open) {
+		regmap_write(phy->apb, SOC_PCIECTRL_CTRL21_ADDR, 0x20000070);
+
+		hi3670_pcie_oe_ctrl(phy, true);
+
+		/* en hard gt mode */
+		regmap_read(phy->crgctrl, CRGPERIPH_PCIECTRL0, &val);
+		val &= ~(0x1 << 0);
+		regmap_write(phy->crgctrl, CRGPERIPH_PCIECTRL0, val);
+
+		/* disable soft gt mode */
+		regmap_write(phy->crgctrl, CRGPERIPH_PERDIS12, 0x2000);
+
+	} else {
+		/* disable hard gt mode */
+		regmap_read(phy->crgctrl, CRGPERIPH_PCIECTRL0, &val);
+		val |= (0x1 << 0);
+		regmap_write(phy->crgctrl, CRGPERIPH_PCIECTRL0, val);
+
+		/* disable soft gt mode */
+		regmap_write(phy->crgctrl, CRGPERIPH_PERDIS12, 0x2000);
+
+		hi3670_pcie_oe_ctrl(phy, false);
+       }
+}
+
+static int hi3670_pcie_allclk_ctrl(struct hi3670_pcie_phy *phy, bool clk_on)
+{
+	struct device *dev = phy->dev;
+	u32 val;
+	int ret = 0;
+
+	if (!clk_on)
+		goto close_clocks;
+
+	/* choose 100MHz clk src: Bit[8]==1 pad, Bit[8]==0 pll */
+	val = hi3670_apb_phy_readl(phy, SOC_PCIEPHY_CTRL1_ADDR);
+	val &= ~(0x1 << 8);
+	hi3670_apb_phy_writel(phy, val, SOC_PCIEPHY_CTRL1_ADDR);
+
+	hi3670_pcie_pll_init(phy);
+
+	ret = hi3670_pcie_pll_ctrl(phy, true);
+	if (ret) {
+		dev_err(dev, "Failed to enable pll\n");
+		return -1;
+	}
+	hi3670_pcie_hp_debounce_gt(phy, true);
+	hi3670_pcie_phyref_gt(phy, true);
+	hi3670_pcie_ioref_gt(phy, true);
+
+	ret = clk_set_rate(phy->aclk, AXI_CLK_FREQ);
+	if (ret) {
+		dev_err(dev, "Failed to set rate\n");
+		goto close_clocks;
+	}
+
+	return 0;
+
+close_clocks:
+	hi3670_pcie_ioref_gt(phy, false);
+	hi3670_pcie_phyref_gt(phy, false);
+	hi3670_pcie_hp_debounce_gt(phy, false);
+
+	hi3670_pcie_pll_ctrl(phy, false);
+
+	return ret;
+}
+
+static bool is_pipe_clk_stable(struct hi3670_pcie_phy *phy)
+{
+	struct device *dev = phy->dev;
+	u32 val;
+	u32 time = 100;
+	u32 pipe_clk_stable = 0x1 << 19;
+
+	val = hi3670_apb_phy_readl(phy, 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 = hi3670_apb_phy_readl(phy, SOC_PCIEPHY_STATE0_ADDR);
+	}
+
+	return true;
+}
+
+static int hi3670_pcie_noc_power(struct hi3670_pcie_phy *phy, bool enable)
+{
+	struct device *dev = phy->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(phy->pmctrl, NOC_POWER_IDLEREQ_1, val);
+
+	time = 100;
+	regmap_read(phy->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(phy->pmctrl, NOC_POWER_IDLE_1, &val);
+	}
+
+	return 0;
+}
+
+static int hi3670_pcie_get_apb(struct hi3670_pcie_phy *phy)
+{
+	struct device_node *pcie_port;
+	struct device *dev = phy->dev;
+	struct device *pcie_dev;
+
+	pcie_port = of_get_child_by_name(dev->parent->of_node, "pcie");
+	if (!pcie_port) {
+		dev_err(dev, "no pcie node found in %s\n",
+			dev->parent->of_node->full_name);
+		return -ENODEV;
+	}
+
+	pcie_dev = bus_find_device_by_of_node(&platform_bus_type, pcie_port);
+	if (!pcie_dev) {
+                dev_err(dev, "Didn't find pcie device\n");
+                return -ENODEV;
+        }
+
+        /*
+	 * We might just use NULL instead of the APB name, as the
+	 * pcie-kirin currently registers directly just one regmap (although
+	 * the DWC driver register other regmaps).
+	 *
+	 * Yet, it sounds safer to warrant that it will be accessing the
+	 * right regmap. So, let's use the named version.
+	 */
+	phy->apb = dev_get_regmap(pcie_dev, "kirin_pcie_apb");
+	if (!phy->apb) {
+		dev_err(dev, "Failed to get APB regmap\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+
+static int kirin_pcie_clk_ctrl(struct hi3670_pcie_phy *phy, bool enable)
+{
+	int ret = 0;
+
+	if (!enable)
+		goto close_clk;
+
+	ret = clk_set_rate(phy->phy_ref_clk, REF_CLK_FREQ);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(phy->phy_ref_clk);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(phy->apb_sys_clk);
+	if (ret)
+		goto apb_sys_fail;
+
+	ret = clk_prepare_enable(phy->apb_phy_clk);
+	if (ret)
+		goto apb_phy_fail;
+
+	ret = clk_prepare_enable(phy->aclk);
+	if (ret)
+		goto aclk_fail;
+
+	ret = clk_prepare_enable(phy->aux_clk);
+	if (ret)
+		goto aux_clk_fail;
+
+	return 0;
+
+close_clk:
+	clk_disable_unprepare(phy->aux_clk);
+aux_clk_fail:
+	clk_disable_unprepare(phy->aclk);
+aclk_fail:
+	clk_disable_unprepare(phy->apb_phy_clk);
+apb_phy_fail:
+	clk_disable_unprepare(phy->apb_sys_clk);
+apb_sys_fail:
+	clk_disable_unprepare(phy->phy_ref_clk);
+
+	return ret;
+}
+
+static int hi3670_pcie_phy_init(struct phy *generic_phy)
+{
+	struct hi3670_pcie_phy *phy = phy_get_drvdata(generic_phy);
+	struct device *dev = phy->dev;
+	int ret;
+
+	/*
+	 * The code under hi3670_pcie_get_apb() need to access the
+	 * DWC APB registers. So, get them from
+	 * the pcie driver's regmap (see pcie-kirin regmap).
+	 *
+	 * Such kind of resource can only be obtained during the PCIe
+	 * power_on sequence, as the code inside pcie-kirin needs to
+	 * be already probed, as it needs to register the APB regmap.
+	 */
+
+	ret = hi3670_pcie_get_apb(phy);
+	if (ret)
+		return ret;
+
+	/* phy regulator needs to be powered on before calling it */
+	return hi3670_pcie_gpio_request(phy, dev);
+}
+
+static int hi3670_pcie_phy_power_on(struct phy *generic_phy)
+{
+	struct hi3670_pcie_phy *phy = phy_get_drvdata(generic_phy);
+	int val, ret, i;
+
+	/* Power supply for Host */
+	regmap_write(phy->sysctrl,
+		     SCTRL_PCIE_CMOS_OFFSET, SCTRL_PCIE_CMOS_BIT);
+	usleep_range(TIME_CMOS_MIN, TIME_CMOS_MAX);
+
+	hi3670_pcie_phy_oe_enable(phy);
+
+	for (i = 0; i < phy->n_gpio_clkreq; i++) {
+		ret = gpio_direction_output(phy->gpio_id_clkreq[i], 0);
+		if (ret)
+			return ret;
+	}
+
+	ret = kirin_pcie_clk_ctrl(phy, true);
+	if (ret)
+		return ret;
+
+	/* ISO disable, PCIeCtrl, PHY assert and clk gate clear */
+	regmap_write(phy->sysctrl,
+		     SCTRL_PCIE_ISO_OFFSET, SCTRL_PCIE_ISO_BIT);
+	regmap_write(phy->crgctrl,
+		     CRGCTRL_PCIE_ASSERT_OFFSET, CRGCTRL_PCIE_ASSERT_BIT);
+	regmap_write(phy->sysctrl,
+		     SCTRL_PCIE_HPCLK_OFFSET, SCTRL_PCIE_HPCLK_BIT);
+
+	hi3670_pcie_natural_cfg(phy);
+
+	ret = hi3670_pcie_allclk_ctrl(phy, true);
+	if (ret)
+		goto disable_clks;
+
+	/* pull down phy_test_powerdown signal */
+	val = hi3670_apb_phy_readl(phy, SOC_PCIEPHY_CTRL0_ADDR);
+	val &= ~(0x1 << 22);
+	hi3670_apb_phy_writel(phy, val, SOC_PCIEPHY_CTRL0_ADDR);
+
+	/* deassert controller perst_n */
+	regmap_read(phy->apb, SOC_PCIECTRL_CTRL12_ADDR, &val);
+	val |= (0x1 << 2);
+	regmap_write(phy->apb, SOC_PCIECTRL_CTRL12_ADDR, val);
+	udelay(10);
+
+	/* perst assert Endpoints */
+	usleep_range(21000, 23000);
+	for (i = 0; i < phy->n_gpio_resets; i++) {
+		ret = gpio_direction_output(phy->gpio_id_reset[i], 1);
+		if (ret)
+			return ret;
+	}
+	usleep_range(10000, 11000);
+
+	ret = is_pipe_clk_stable(phy);
+	if (!ret)
+		goto disable_clks;
+
+	hi3670_pcie_set_eyeparam(phy);
+
+	ret = hi3670_pcie_noc_power(phy, false);
+	if (ret)
+		goto disable_clks;
+
+	return 0;
+
+disable_clks:
+	kirin_pcie_clk_ctrl(phy, false);
+	return ret;
+}
+
+static int hi3670_pcie_phy_power_off(struct phy *generic_phy)
+{
+	struct hi3670_pcie_phy *phy = phy_get_drvdata(generic_phy);
+
+	/* Drop power supply for Host */
+	regmap_write(phy->sysctrl, SCTRL_PCIE_CMOS_OFFSET, 0x00);
+
+	kirin_pcie_clk_ctrl(phy, false);
+
+	return 0;
+}
+
+static const struct phy_ops hi3670_phy_ops = {
+	.init		= hi3670_pcie_phy_init,
+	.power_on	= hi3670_pcie_phy_power_on,
+	.power_off	= hi3670_pcie_phy_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static int hi3670_pcie_phy_get_resources(struct hi3670_pcie_phy *phy,
+					 struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	char name[32];
+	int i;
+
+	/* syscon */
+	phy->crgctrl = syscon_regmap_lookup_by_compatible("hisilicon,hi3670-crgctrl");
+	if (IS_ERR(phy->crgctrl))
+		return PTR_ERR(phy->crgctrl);
+
+	phy->sysctrl = syscon_regmap_lookup_by_compatible("hisilicon,hi3670-sctrl");
+	if (IS_ERR(phy->sysctrl))
+		return PTR_ERR(phy->sysctrl);
+
+	phy->pmctrl = syscon_regmap_lookup_by_compatible("hisilicon,hi3670-pmctrl");
+	if (IS_ERR(phy->sysctrl))
+		return PTR_ERR(phy->sysctrl);
+
+	/* clocks */
+	phy->phy_ref_clk = devm_clk_get(dev, "phy_ref");
+	if (IS_ERR(phy->phy_ref_clk))
+		return PTR_ERR(phy->phy_ref_clk);
+
+	phy->aux_clk = devm_clk_get(dev, "aux");
+	if (IS_ERR(phy->aux_clk))
+		return PTR_ERR(phy->aux_clk);
+
+	phy->apb_phy_clk = devm_clk_get(dev, "apb_phy");
+	if (IS_ERR(phy->apb_phy_clk))
+		return PTR_ERR(phy->apb_phy_clk);
+
+	phy->apb_sys_clk = devm_clk_get(dev, "apb_sys");
+	if (IS_ERR(phy->apb_sys_clk))
+		return PTR_ERR(phy->apb_sys_clk);
+
+	phy->aclk = devm_clk_get(dev, "aclk");
+	if (IS_ERR(phy->aclk))
+		return PTR_ERR(phy->aclk);
+
+	/* registers */
+	phy->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(phy->base))
+		return PTR_ERR(phy->base);
+
+	/* perst reset gpios */
+	phy->n_gpio_resets = of_gpio_named_count(np, "reset-gpios");
+	if (phy->n_gpio_resets > MAX_GPIO_RESETS) {
+		dev_err(dev, "Too many GPIO resets!\n");
+		return -EINVAL;
+	}
+	for (i = 0; i < phy->n_gpio_resets; i++) {
+		phy->gpio_id_reset[i] = of_get_named_gpio(dev->of_node,
+							  "reset-gpios", i);
+		if (phy->gpio_id_reset[i] < 0)
+			return phy->gpio_id_reset[i];
+
+		sprintf(name, "pcie_perst_%d", i);
+
+		phy->reset_names[i] = devm_kstrdup_const(dev, name,
+							 GFP_KERNEL);
+		if (!phy->reset_names[i])
+			return -ENOMEM;
+	}
+
+	/* clock request gpios */
+	phy->n_gpio_clkreq = of_gpio_named_count(np, "clkreq-gpios");
+	if (phy->n_gpio_clkreq > MAX_GPIO_CLKREQ) {
+		dev_err(dev, "Too many GPIO clock requests!\n");
+		return -EINVAL;
+	}
+	for (i = 0; i < phy->n_gpio_clkreq; i++) {
+		phy->gpio_id_clkreq[i] = of_get_named_gpio(dev->of_node,
+							   "clkreq-gpios", i);
+		if (phy->gpio_id_clkreq[i] < 0)
+			return phy->gpio_id_clkreq[i];
+
+		sprintf(name, "pcie_clkreq_%d", i);
+		phy->clkreq_names[i] = devm_kstrdup_const(dev, name,
+							  GFP_KERNEL);
+		if (!phy->clkreq_names[i])
+			return -ENOMEM;
+	}
+
+	hi3670_pcie_get_eyeparam(phy);
+
+	return 0;
+}
+
+static int hi3670_pcie_phy_probe(struct platform_device *pdev)
+{
+	struct phy_provider *phy_provider;
+	struct device *dev = &pdev->dev;
+	struct hi3670_pcie_phy *phy;
+	struct phy *generic_phy;
+	int ret;
+
+	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
+	if (!phy)
+		return -ENOMEM;
+
+	phy->dev = dev;
+
+	ret = hi3670_pcie_phy_get_resources(phy, pdev);
+	if (ret)
+		return ret;
+
+	generic_phy = devm_phy_create(dev, dev->of_node, &hi3670_phy_ops);
+	if (IS_ERR(generic_phy)) {
+		dev_err(dev, "failed to create PHY\n");
+		return PTR_ERR(generic_phy);
+	}
+
+	phy_set_drvdata(generic_phy, phy);
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+
+	return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static const struct of_device_id hi3670_pcie_phy_match[] = {
+	{
+		.compatible = "hisilicon,hi970-pcie-phy",
+	},
+	{},
+};
+
+static struct platform_driver hi3670_pcie_phy_driver = {
+	.probe	= hi3670_pcie_phy_probe,
+	.driver = {
+		.of_match_table	= hi3670_pcie_phy_match,
+		.name		= "hi3670_pcie_phy",
+		.suppress_bind_attrs = true,
+	}
+};
+builtin_platform_driver(hi3670_pcie_phy_driver);
+
+MODULE_DEVICE_TABLE(of, hi3670_pcie_phy_match);
+MODULE_DESCRIPTION("PCIe phy driver for Kirin 970");
+MODULE_AUTHOR("Mauro Carvalho Chehab <mchehab@kernel.org>");
+MODULE_AUTHOR("Manivannan Sadhasivam <mani@kernel.org>");
+MODULE_LICENSE("GPL v2");
-- 
2.31.1

[PATCH v7 08/10] arm64: dts: HiSilicon: Add support for HiKey 970 PCIe controller hardware

[Mauro Carvalho Chehab]

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

Add DTS bindings for the HiKey 970 board's PCIe hardware.

Co-developed-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 arch/arm64/boot/dts/hisilicon/hi3670.dtsi     | 71 +++++++++++++++++++
 .../boot/dts/hisilicon/hikey970-pmic.dtsi     |  1 -
 drivers/pci/controller/dwc/pcie-kirin.c       | 12 ----
 3 files changed, 71 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
index 1f228612192c..6dfcfcfeedae 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
@@ -177,6 +177,12 @@ sctrl: sctrl@fff0a000 {
 			#clock-cells = <1>;
 		};
 
+		pmctrl: pmctrl@fff31000 {
+			compatible = "hisilicon,hi3670-pmctrl", "syscon";
+			reg = <0x0 0xfff31000 0x0 0x1000>;
+			#clock-cells = <1>;
+		};
+
 		iomcu: iomcu@ffd7e000 {
 			compatible = "hisilicon,hi3670-iomcu", "syscon";
 			reg = <0x0 0xffd7e000 0x0 0x1000>;
@@ -660,6 +666,71 @@ gpio28: gpio@fff1d000 {
 			clock-names = "apb_pclk";
 		};
 
+		its_pcie: interrupt-controller@f4000000 {
+			compatible = "arm,gic-v3-its";
+			msi-controller;
+			reg = <0x0 0xf5100000 0x0 0x100000>;
+		};
+
+		pcie_phy: pcie-phy@fc000000 {
+			compatible = "hisilicon,hi970-pcie-phy";
+			reg = <0x0 0xfc000000 0x0 0x80000>;
+
+			phy-supply = <&ldo33>;
+
+			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 = "phy_ref", "aux",
+				      "apb_phy", "apb_sys",
+				      "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 */
+			hisilicon,eye-diagram-param = <0xFFFFFFFF 0xFFFFFFFF
+						       0xFFFFFFFF 0xFFFFFFFF
+						       0xFFFFFFFF>;
+
+			#phy-cells = <0>;
+		};
+
+		pcie@f4000000 {
+			compatible = "hisilicon,kirin970-pcie";
+			reg = <0x0 0xf4000000 0x0 0x1000000>,
+			      <0x0 0xfc180000 0x0 0x1000>,
+			      <0x0 0xf5000000 0x0 0x2000>;
+			reg-names = "dbi", "apb", "config";
+			bus-range = <0x0  0x1>;
+			msi-parent = <&its_pcie>;
+			#address-cells = <3>;
+			#size-cells = <2>;
+			device_type = "pci";
+			phys = <&pcie_phy>;
+			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>;
+		};
+
 		/* UFS */
 		ufs: ufs@ff3c0000 {
 			compatible = "hisilicon,hi3670-ufs", "jedec,ufs-2.1";
diff --git a/arch/arm64/boot/dts/hisilicon/hikey970-pmic.dtsi b/arch/arm64/boot/dts/hisilicon/hikey970-pmic.dtsi
index 48c739eacba0..03452e627641 100644
--- a/arch/arm64/boot/dts/hisilicon/hikey970-pmic.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hikey970-pmic.dtsi
@@ -73,7 +73,6 @@ ldo33: LDO33 { /* PEX8606 */
 					regulator-name = "ldo33";
 					regulator-min-microvolt = <2500000>;
 					regulator-max-microvolt = <3300000>;
-					regulator-boot-on;
 				};
 
 				ldo34: LDO34 { /* GPS AUX IN VDD */
diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index bfc0513f7b15..9dad14929538 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -347,18 +347,6 @@ static const struct regmap_config pcie_kirin_regmap_conf = {
 	.reg_stride = 4,
 };
 
-/* Registers in PCIeCTRL */
-static inline void kirin_apb_ctrl_writel(struct kirin_pcie *kirin_pcie,
-					 u32 val, u32 reg)
-{
-	writel(val, kirin_pcie->apb_base + reg);
-}
-
-static inline u32 kirin_apb_ctrl_readl(struct kirin_pcie *kirin_pcie, u32 reg)
-{
-	return readl(kirin_pcie->apb_base + reg);
-}
-
 static long kirin_pcie_get_resource(struct kirin_pcie *kirin_pcie,
 				    struct platform_device *pdev)
 {
-- 
2.31.1

[PATCH v7 09/10] dt-bindings: PCI: kirin-pcie.txt: Convert it to yaml

[Mauro Carvalho Chehab]

Convert the file into a JSON description at the yaml format.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 .../bindings/pci/hisilicon,kirin-pcie.yaml    | 87 +++++++++++++++++++
 .../devicetree/bindings/pci/kirin-pcie.txt    | 50 -----------
 .../devicetree/bindings/pci/snps,dw-pcie.yaml |  2 +-
 MAINTAINERS                                   |  2 +-
 4 files changed, 89 insertions(+), 52 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..eabc651c9766
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
@@ -0,0 +1,87 @@
+# 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 and
+  inherits common properties defined in
+  Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml.
+
+allOf:
+  - $ref: /schemas/pci/snps,dw-pcie.yaml#
+
+properties:
+  compatible:
+    contains:
+      enum:
+        - hisilicon,kirin960-pcie
+        - hisilicon,kirin970-pcie
+
+  reg:
+    description: |
+      Should contain rc_dbi, apb, config registers location and length.
+    minItems: 3
+    maxItems: 4
+
+  reg-names:
+    items:
+      - const: dbi          # controller configuration registers
+      - const: apb          # apb Ctrl register defined by Kirin
+      - const: config       # PCIe configuration space registers
+      - const: phy          # apb PHY register used on Kirin 960 PHY
+    minItems: 3
+    maxItems: 4
+
+  reset-gpios:
+    description: The GPIO(s) to generate PCIe PERST# assert and deassert signal.
+    minItems: 1
+    maxItems: 4
+
+required:
+  - compatible
+  - reg
+  - reg-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      pcie: pcie@f4000000 {
+        compatible = "hisilicon,kirin970-pcie";
+        reg = <0x0 0xf4000000 0x0 0x1000>,
+              <0x0 0xff3fe000 0x0 0x1000>,
+              <0x0 0xf4000000 0 0x2000>;
+        reg-names = "dbi", "apb", "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>;
+        reset-gpios = <&gpio7 0 0 >, <&gpio25 2 0 >,
+                      <&gpio3 1 0 >, <&gpio27 4 0 >;
+      };
+    };
diff --git a/Documentation/devicetree/bindings/pci/kirin-pcie.txt b/Documentation/devicetree/bindings/pci/kirin-pcie.txt
deleted file mode 100644
index 7adab8999a6a..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,dw-pcie.yaml.
-
-Additional properties are described here:
-
-Required properties
-- compatible:
-	"hisilicon,kirin960-pcie"
-- 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,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>;
-		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/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
index a8c1db879fb9..d80894a5abf5 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
@@ -34,7 +34,7 @@ properties:
     minItems: 2
     maxItems: 5
     items:
-      enum: [dbi, dbi2, config, atu, app, elbi, mgmt, ctrl, parf, cfg, link]
+      enum: [dbi, dbi2, config, atu, apb, app, elbi, mgmt, ctrl, parf, cfg, link]
 
   num-lanes:
     description: |
diff --git a/MAINTAINERS b/MAINTAINERS
index b54bd9dd07ec..d5f53b2d3f9c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14420,7 +14420,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.31.1

[PATCH v7 11/10] PCI: kirin: Allow building it as a module

[Mauro Carvalho Chehab]

There's nothing preventing this driver to be loaded as a
module. So, change its config from bool to tristate.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/pci/controller/dwc/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 423d35872ce4..e0091bfae5b5 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -227,7 +227,7 @@ config PCIE_INTEL_GW
 
 config PCIE_KIRIN
 	depends on OF && (ARM64 || COMPILE_TEST)
-	bool "HiSilicon Kirin series SoCs PCIe controllers"
+	tristate "HiSilicon Kirin series SoCs PCIe controllers"
 	depends on PCI_MSI_IRQ_DOMAIN
 	select PCIE_DW_HOST
 	help
-- 
2.31.1

Re: [PATCH v7 11/10] PCI: kirin: Allow building it as a module

[Arnd Bergmann]

On Wed, Jul 21, 2021 at 12:15 PM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> There's nothing preventing this driver to be loaded as a
> module. So, change its config from bool to tristate.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

No objections from me, but I wonder if you would also consider making the
module removable. It's currently marked as 'builtin_platform_driver',
so you can load but not remove it. Rob has done some bug fixes that make
it possible to remove similar drivers, so it's probably not much work
here either.

     Arnd

Re: [PATCH v7 11/10] PCI: kirin: Allow building it as a module

[Mauro Carvalho Chehab]

Em Wed, 21 Jul 2021 13:55:07 +0200
Arnd Bergmann <arnd@arndb.de> escreveu:

> On Wed, Jul 21, 2021 at 12:15 PM Mauro Carvalho Chehab
> <mchehab+huawei@kernel.org> wrote:
> >
> > There's nothing preventing this driver to be loaded as a
> > module. So, change its config from bool to tristate.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>  
> 
> No objections from me, but I wonder if you would also consider making the
> module removable. It's currently marked as 'builtin_platform_driver',
> so you can load but not remove it. Rob has done some bug fixes that make
> it possible to remove similar drivers, so it's probably not much work
> here either.

Yeah, I can probably work on a patch to unbind/remove this driver.

Never actually tried to write a patch removing the PCIe BUS, so no
idea if the refcounts for the in-board Ethernet NIC, M.2 and mini-PCIe
slots will be properly handled. If refcount is handled properly, I
guess a patch like that won't be hard, at least for Kirin 970 PHY.

The Kirin 960 PHY will require a small change at the current version,
as it currently misses the power_off logic.

I also need to double-check if devm resources will be freed at the 
driver removal time, as, with some past tests with media devices,
we had some issues with that.

Thanks,
Mauro

Re: [PATCH v7 08/10] arm64: dts: HiSilicon: Add support for HiKey 970 PCIe controller hardware

[Manivannan Sadhasivam]

On Wed, Jul 21, 2021 at 10:39:10AM +0200, Mauro Carvalho Chehab wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> Add DTS bindings for the HiKey 970 board's PCIe hardware.
> 
> Co-developed-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>  arch/arm64/boot/dts/hisilicon/hi3670.dtsi     | 71 +++++++++++++++++++
>  .../boot/dts/hisilicon/hikey970-pmic.dtsi     |  1 -
>  drivers/pci/controller/dwc/pcie-kirin.c       | 12 ----
>  3 files changed, 71 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
> index 1f228612192c..6dfcfcfeedae 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
> @@ -177,6 +177,12 @@ sctrl: sctrl@fff0a000 {
>  			#clock-cells = <1>;
>  		};
>  
> +		pmctrl: pmctrl@fff31000 {
> +			compatible = "hisilicon,hi3670-pmctrl", "syscon";
> +			reg = <0x0 0xfff31000 0x0 0x1000>;
> +			#clock-cells = <1>;
> +		};
> +

Irrelevant change to this patch.

>  		iomcu: iomcu@ffd7e000 {
>  			compatible = "hisilicon,hi3670-iomcu", "syscon";
>  			reg = <0x0 0xffd7e000 0x0 0x1000>;
> @@ -660,6 +666,71 @@ gpio28: gpio@fff1d000 {
>  			clock-names = "apb_pclk";
>  		};
>  

[...]

> +			#interrupt-cells = <1>;
> +			interrupts = <0 283 4>;

Use the DT flag for interrupts instead of hardcoded value

> +			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>;
> +		};
> +
>  		/* UFS */
>  		ufs: ufs@ff3c0000 {
>  			compatible = "hisilicon,hi3670-ufs", "jedec,ufs-2.1";
> diff --git a/arch/arm64/boot/dts/hisilicon/hikey970-pmic.dtsi b/arch/arm64/boot/dts/hisilicon/hikey970-pmic.dtsi
> index 48c739eacba0..03452e627641 100644
> --- a/arch/arm64/boot/dts/hisilicon/hikey970-pmic.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hikey970-pmic.dtsi
> @@ -73,7 +73,6 @@ ldo33: LDO33 { /* PEX8606 */
>  					regulator-name = "ldo33";
>  					regulator-min-microvolt = <2500000>;
>  					regulator-max-microvolt = <3300000>;
> -					regulator-boot-on;

Again, irrelevant.

>  				};
>  
>  				ldo34: LDO34 { /* GPS AUX IN VDD */
> diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
> index bfc0513f7b15..9dad14929538 100644
> --- a/drivers/pci/controller/dwc/pcie-kirin.c
> +++ b/drivers/pci/controller/dwc/pcie-kirin.c
> @@ -347,18 +347,6 @@ static const struct regmap_config pcie_kirin_regmap_conf = {
>  	.reg_stride = 4,
>  };
>  
> -/* Registers in PCIeCTRL */
> -static inline void kirin_apb_ctrl_writel(struct kirin_pcie *kirin_pcie,
> -					 u32 val, u32 reg)
> -{
> -	writel(val, kirin_pcie->apb_base + reg);
> -}
> -
> -static inline u32 kirin_apb_ctrl_readl(struct kirin_pcie *kirin_pcie, u32 reg)
> -{
> -	return readl(kirin_pcie->apb_base + reg);
> -}
> -

Same here...

Thanks,
Mani

Re: [PATCH v7 08/10] arm64: dts: HiSilicon: Add support for HiKey 970 PCIe controller hardware

[Mauro Carvalho Chehab]

Em Thu, 22 Jul 2021 19:06:28 +0530
Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> escreveu:

> On Wed, Jul 21, 2021 at 10:39:10AM +0200, Mauro Carvalho Chehab wrote:
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > 
> > Add DTS bindings for the HiKey 970 board's PCIe hardware.
> > 
> > Co-developed-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > ---
> >  arch/arm64/boot/dts/hisilicon/hi3670.dtsi     | 71 +++++++++++++++++++
> >  .../boot/dts/hisilicon/hikey970-pmic.dtsi     |  1 -
> >  drivers/pci/controller/dwc/pcie-kirin.c       | 12 ----
> >  3 files changed, 71 insertions(+), 13 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
> > index 1f228612192c..6dfcfcfeedae 100644
> > --- a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
> > +++ b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
> > @@ -177,6 +177,12 @@ sctrl: sctrl@fff0a000 {
> >  			#clock-cells = <1>;
> >  		};
> >  
> > +		pmctrl: pmctrl@fff31000 {
> > +			compatible = "hisilicon,hi3670-pmctrl", "syscon";
> > +			reg = <0x0 0xfff31000 0x0 0x1000>;
> > +			#clock-cells = <1>;
> > +		};
> > +  
> 
> Irrelevant change to this patch.

Huh?

This is used by PCIe PHY, as part of the power on procedures:

	+static int hi3670_pcie_noc_power(struct hi3670_pcie_phy *phy, bool enable)
	+{
	+       struct device *dev = phy->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(phy->pmctrl, NOC_POWER_IDLEREQ_1, val);



> 
> >  		iomcu: iomcu@ffd7e000 {
> >  			compatible = "hisilicon,hi3670-iomcu", "syscon";
> >  			reg = <0x0 0xffd7e000 0x0 0x1000>;
> > @@ -660,6 +666,71 @@ gpio28: gpio@fff1d000 {
> >  			clock-names = "apb_pclk";
> >  		};
> >    
> 
> [...]
> 
> > +			#interrupt-cells = <1>;
> > +			interrupts = <0 283 4>;  
> 
> Use the DT flag for interrupts instead of hardcoded value

Do you mean like this?

	interrupts = <0 283 IRQ_TYPE_LEVEL_HIGH>;

> 
> > +			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>;
> > +		};
> > +
> >  		/* UFS */
> >  		ufs: ufs@ff3c0000 {
> >  			compatible = "hisilicon,hi3670-ufs", "jedec,ufs-2.1";
> > diff --git a/arch/arm64/boot/dts/hisilicon/hikey970-pmic.dtsi b/arch/arm64/boot/dts/hisilicon/hikey970-pmic.dtsi
> > index 48c739eacba0..03452e627641 100644
> > --- a/arch/arm64/boot/dts/hisilicon/hikey970-pmic.dtsi
> > +++ b/arch/arm64/boot/dts/hisilicon/hikey970-pmic.dtsi
> > @@ -73,7 +73,6 @@ ldo33: LDO33 { /* PEX8606 */
> >  					regulator-name = "ldo33";
> >  					regulator-min-microvolt = <2500000>;
> >  					regulator-max-microvolt = <3300000>;
> > -					regulator-boot-on;  
> 
> Again, irrelevant.

I'll move it to the USB patch series, where the PMIC is added.

> 
> >  				};
> >  
> >  				ldo34: LDO34 { /* GPS AUX IN VDD */
> > diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
> > index bfc0513f7b15..9dad14929538 100644
> > --- a/drivers/pci/controller/dwc/pcie-kirin.c
> > +++ b/drivers/pci/controller/dwc/pcie-kirin.c
> > @@ -347,18 +347,6 @@ static const struct regmap_config pcie_kirin_regmap_conf = {
> >  	.reg_stride = 4,
> >  };
> >  
> > -/* Registers in PCIeCTRL */
> > -static inline void kirin_apb_ctrl_writel(struct kirin_pcie *kirin_pcie,
> > -					 u32 val, u32 reg)
> > -{
> > -	writel(val, kirin_pcie->apb_base + reg);
> > -}
> > -
> > -static inline u32 kirin_apb_ctrl_readl(struct kirin_pcie *kirin_pcie, u32 reg)
> > -{
> > -	return readl(kirin_pcie->apb_base + reg);
> > -}
> > -  
> 
> Same here...

This hunk should be on patch 03/10. Probably some rebase added it here by
mistake. I'll fix it on v8.

Thanks,
Mauro

Re: [PATCH v7 09/10] dt-bindings: PCI: kirin-pcie.txt: Convert it to yaml

[Rob Herring]

On Wed, Jul 21, 2021 at 10:39:11AM +0200, Mauro Carvalho Chehab wrote:
> Convert the file into a JSON description at the yaml format.

And add 970...

> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>  .../bindings/pci/hisilicon,kirin-pcie.yaml    | 87 +++++++++++++++++++
>  .../devicetree/bindings/pci/kirin-pcie.txt    | 50 -----------
>  .../devicetree/bindings/pci/snps,dw-pcie.yaml |  2 +-
>  MAINTAINERS                                   |  2 +-
>  4 files changed, 89 insertions(+), 52 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..eabc651c9766
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/hisilicon,kirin-pcie.yaml
> @@ -0,0 +1,87 @@
> +# 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 and
> +  inherits common properties defined in
> +  Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml.
> +
> +allOf:
> +  - $ref: /schemas/pci/snps,dw-pcie.yaml#
> +
> +properties:
> +  compatible:
> +    contains:
> +      enum:
> +        - hisilicon,kirin960-pcie
> +        - hisilicon,kirin970-pcie
> +
> +  reg:
> +    description: |
> +      Should contain rc_dbi, apb, config registers location and length.
> +    minItems: 3
> +    maxItems: 4
> +
> +  reg-names:
> +    items:
> +      - const: dbi          # controller configuration registers
> +      - const: apb          # apb Ctrl register defined by Kirin
> +      - const: config       # PCIe configuration space registers
> +      - const: phy          # apb PHY register used on Kirin 960 PHY
> +    minItems: 3
> +    maxItems: 4
> +
> +  reset-gpios:
> +    description: The GPIO(s) to generate PCIe PERST# assert and deassert signal.
> +    minItems: 1
> +    maxItems: 4

I'll apply this, but only with 'maxItems: 1' if you want to separate the 
discussion on that part.

> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    soc {
> +      #address-cells = <2>;
> +      #size-cells = <2>;
> +
> +      pcie: pcie@f4000000 {
> +        compatible = "hisilicon,kirin970-pcie";
> +        reg = <0x0 0xf4000000 0x0 0x1000>,
> +              <0x0 0xff3fe000 0x0 0x1000>,
> +              <0x0 0xf4000000 0 0x2000>;
> +        reg-names = "dbi", "apb", "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>;
> +        reset-gpios = <&gpio7 0 0 >, <&gpio25 2 0 >,
> +                      <&gpio3 1 0 >, <&gpio27 4 0 >;
> +      };
> +    };

Re: [PATCH v7 08/10] arm64: dts: HiSilicon: Add support for HiKey 970 PCIe controller hardware

[Manivannan Sadhasivam]

On Fri, Jul 23, 2021 at 08:53:18AM +0200, Mauro Carvalho Chehab wrote:
> Em Thu, 22 Jul 2021 19:06:28 +0530
> Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> escreveu:
> 
> > On Wed, Jul 21, 2021 at 10:39:10AM +0200, Mauro Carvalho Chehab wrote:
> > > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > 
> > > Add DTS bindings for the HiKey 970 board's PCIe hardware.
> > > 
> > > Co-developed-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > > ---
> > >  arch/arm64/boot/dts/hisilicon/hi3670.dtsi     | 71 +++++++++++++++++++
> > >  .../boot/dts/hisilicon/hikey970-pmic.dtsi     |  1 -
> > >  drivers/pci/controller/dwc/pcie-kirin.c       | 12 ----
> > >  3 files changed, 71 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
> > > index 1f228612192c..6dfcfcfeedae 100644
> > > --- a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
> > > +++ b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
> > > @@ -177,6 +177,12 @@ sctrl: sctrl@fff0a000 {
> > >  			#clock-cells = <1>;
> > >  		};
> > >  
> > > +		pmctrl: pmctrl@fff31000 {
> > > +			compatible = "hisilicon,hi3670-pmctrl", "syscon";
> > > +			reg = <0x0 0xfff31000 0x0 0x1000>;
> > > +			#clock-cells = <1>;
> > > +		};
> > > +  
> > 
> > Irrelevant change to this patch.
> 
> Huh?
> 
> This is used by PCIe PHY, as part of the power on procedures:
> 
> 	+static int hi3670_pcie_noc_power(struct hi3670_pcie_phy *phy, bool enable)
> 	+{
> 	+       struct device *dev = phy->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(phy->pmctrl, NOC_POWER_IDLEREQ_1, val);
> 
> 

Ah... you're hardcoding the syscon compatible in driver. Sorry missed that.

But if these syscon nodes are independent memory regions or belong to non
PCI/PHY memory map, you could've fetched the reference through a DT property
along with the offset then used it in driver.

Like,

	pcie_phy: pcie-phy@fc000000 {
		...
		hisilicon,noc-power-regs = <&pmctrl 0x38c>;
		hisilicon,sctrl-cmos-regs = <&sctrl 0x60>;
		...
	};

The benefit of doing this way is, if the pmctrl, sctrl register layout changes
in future, you can handle it without any issues.

> 
> > 
> > >  		iomcu: iomcu@ffd7e000 {
> > >  			compatible = "hisilicon,hi3670-iomcu", "syscon";
> > >  			reg = <0x0 0xffd7e000 0x0 0x1000>;
> > > @@ -660,6 +666,71 @@ gpio28: gpio@fff1d000 {
> > >  			clock-names = "apb_pclk";
> > >  		};
> > >    
> > 
> > [...]
> > 
> > > +			#interrupt-cells = <1>;
> > > +			interrupts = <0 283 4>;  
> > 
> > Use the DT flag for interrupts instead of hardcoded value
> 
> Do you mean like this?
> 
> 	interrupts = <0 283 IRQ_TYPE_LEVEL_HIGH>;
> 

yes but you could also use,

	interrupts = <GIC_SPI 283 IRQ_TYPE_LEVEL_HIGH>;

Thanks,
Mani

Re: [PATCH v7 08/10] arm64: dts: HiSilicon: Add support for HiKey 970 PCIe controller hardware

[Mauro Carvalho Chehab]

Em Sat, 24 Jul 2021 09:41:50 +0530
Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> escreveu:

> On Fri, Jul 23, 2021 at 08:53:18AM +0200, Mauro Carvalho Chehab wrote:
> > Em Thu, 22 Jul 2021 19:06:28 +0530
> > Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> escreveu:
> >   
> > > On Wed, Jul 21, 2021 at 10:39:10AM +0200, Mauro Carvalho Chehab wrote:  
> > > > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > 
> > > > Add DTS bindings for the HiKey 970 board's PCIe hardware.
> > > > 
> > > > Co-developed-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > > > ---
> > > >  arch/arm64/boot/dts/hisilicon/hi3670.dtsi     | 71 +++++++++++++++++++
> > > >  .../boot/dts/hisilicon/hikey970-pmic.dtsi     |  1 -
> > > >  drivers/pci/controller/dwc/pcie-kirin.c       | 12 ----
> > > >  3 files changed, 71 insertions(+), 13 deletions(-)
> > > > 
> > > > diff --git a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
> > > > index 1f228612192c..6dfcfcfeedae 100644
> > > > --- a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
> > > > +++ b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
> > > > @@ -177,6 +177,12 @@ sctrl: sctrl@fff0a000 {
> > > >  			#clock-cells = <1>;
> > > >  		};
> > > >  
> > > > +		pmctrl: pmctrl@fff31000 {
> > > > +			compatible = "hisilicon,hi3670-pmctrl", "syscon";
> > > > +			reg = <0x0 0xfff31000 0x0 0x1000>;
> > > > +			#clock-cells = <1>;
> > > > +		};
> > > > +    
> > > 
> > > Irrelevant change to this patch.  
> > 
> > Huh?
> > 
> > This is used by PCIe PHY, as part of the power on procedures:
> > 
> > 	+static int hi3670_pcie_noc_power(struct hi3670_pcie_phy *phy, bool enable)
> > 	+{
> > 	+       struct device *dev = phy->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(phy->pmctrl, NOC_POWER_IDLEREQ_1, val);
> > 
> >   
> 
> Ah... you're hardcoding the syscon compatible in driver. Sorry missed that.
> 
> But if these syscon nodes are independent memory regions or belong to non
> PCI/PHY memory map, you could've fetched the reference through a DT property
> along with the offset then used it in driver.
> 
> Like,
> 
> 	pcie_phy: pcie-phy@fc000000 {
> 		...
> 		hisilicon,noc-power-regs = <&pmctrl 0x38c>;
> 		hisilicon,sctrl-cmos-regs = <&sctrl 0x60>;
> 		...
> 	};
> 
> The benefit of doing this way is, if the pmctrl, sctrl register layout changes
> in future, you can handle it without any issues.

Interesting approach, but probably overkill. I mean, the register mapping
here should be the same for all Kirin 970 PHY based devices. A PHY for a 
different SoC will likely have other differences than just those two regs.

Regards,
Mauro

Re: [PATCH v7 08/10] arm64: dts: HiSilicon: Add support for HiKey 970 PCIe controller hardware

[Rob Herring]

On Wed, Jul 21, 2021 at 3:39 AM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>
> Add DTS bindings for the HiKey 970 board's PCIe hardware.
>
> Co-developed-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>  arch/arm64/boot/dts/hisilicon/hi3670.dtsi     | 71 +++++++++++++++++++
>  .../boot/dts/hisilicon/hikey970-pmic.dtsi     |  1 -
>  drivers/pci/controller/dwc/pcie-kirin.c       | 12 ----
>  3 files changed, 71 insertions(+), 13 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
> index 1f228612192c..6dfcfcfeedae 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
> @@ -177,6 +177,12 @@ sctrl: sctrl@fff0a000 {
>                         #clock-cells = <1>;
>                 };
>
> +               pmctrl: pmctrl@fff31000 {
> +                       compatible = "hisilicon,hi3670-pmctrl", "syscon";
> +                       reg = <0x0 0xfff31000 0x0 0x1000>;
> +                       #clock-cells = <1>;
> +               };
> +
>                 iomcu: iomcu@ffd7e000 {
>                         compatible = "hisilicon,hi3670-iomcu", "syscon";
>                         reg = <0x0 0xffd7e000 0x0 0x1000>;
> @@ -660,6 +666,71 @@ gpio28: gpio@fff1d000 {
>                         clock-names = "apb_pclk";
>                 };
>
> +               its_pcie: interrupt-controller@f4000000 {
> +                       compatible = "arm,gic-v3-its";
> +                       msi-controller;
> +                       reg = <0x0 0xf5100000 0x0 0x100000>;

How does this h/w have a GIC-400 (which is GICv2) and then a GIC v3 ITS?

> +               };
> +
> +               pcie_phy: pcie-phy@fc000000 {
> +                       compatible = "hisilicon,hi970-pcie-phy";
> +                       reg = <0x0 0xfc000000 0x0 0x80000>;
> +
> +                       phy-supply = <&ldo33>;
> +
> +                       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 = "phy_ref", "aux",
> +                                     "apb_phy", "apb_sys",
> +                                     "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 */
> +                       hisilicon,eye-diagram-param = <0xFFFFFFFF 0xFFFFFFFF
> +                                                      0xFFFFFFFF 0xFFFFFFFF
> +                                                      0xFFFFFFFF>;
> +
> +                       #phy-cells = <0>;
> +               };
> +
> +               pcie@f4000000 {
> +                       compatible = "hisilicon,kirin970-pcie";
> +                       reg = <0x0 0xf4000000 0x0 0x1000000>,
> +                             <0x0 0xfc180000 0x0 0x1000>,
> +                             <0x0 0xf5000000 0x0 0x2000>;
> +                       reg-names = "dbi", "apb", "config";
> +                       bus-range = <0x0  0x1>;
> +                       msi-parent = <&its_pcie>;

This means the PCI host doesn't have a MSI controller...

> +                       #address-cells = <3>;
> +                       #size-cells = <2>;
> +                       device_type = "pci";
> +                       phys = <&pcie_phy>;
> +                       ranges = <0x02000000 0x0 0x00000000
> +                                 0x0 0xf6000000
> +                                 0x0 0x02000000>;
> +                       num-lanes = <1>;
> +                       #interrupt-cells = <1>;
> +                       interrupts = <0 283 4>;
> +                       interrupt-names = "msi";

But then this says it does...