[PATCH v12 00/11] Add support for Hikey 970 PCIe

[PATCH v12 00/11] Add support for Hikey 970 PCIe

[Mauro Carvalho Chehab]

The pcie-kirin PCIe driver contains internally a PHY interface for
Kirin 960, but it misses support for Kirin 970.

Patch1 contains a PHY for Kirin 970 PCIe.

The remaining patches add support for Kirin 970 at the pcie-kirin driver, and
add the needed logic to compile it as module and to allow to dynamically
remove the driver in runtime.

Tested on HiKey970:

  # lspci -D -PP
  0000:00:00.0 PCI bridge: Huawei Technologies Co., Ltd. Device 3670 (rev 01)
  0000:00:00.0/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)
  0000:00:00.0/01:00.0/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)
  0000:00:00.0/01:00.0/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)
  0000:00:00.0/01:00.0/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)
  0000:00:00.0/01:00.0/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)
  0000:00:00.0/01:00.0/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)
  0000:00:00.0/01:00.0/02:01.0/03:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd Device a809
  0000:00:00.0/01:00.0/02:07.0/06:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 07)

Tested on HiKey960:

  # lspci -D 
  0000:00:00.0 PCI bridge: Huawei Technologies Co., Ltd. Device 3660 (rev 01)

---

v12:
  - Change a comment at patch 1 to not use c99 style.

v11:
  - patch 5 changed to use the right PCIe topology
  - all other patches are identical to v10.

v10:
  - patch 1: dropped magic numbers from PHY driver
  - patch 5: allow pcie child nodes without reset-gpios
  - all other patches are identical to v9.

v9:
  - Did some cleanups at patches 1 and 5



Mauro Carvalho Chehab (11):
  phy: HiSilicon: Add driver for Kirin 970 PCIe PHY
  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 support for bridge slot DT schema
  PCI: kirin: Add Kirin 970 compatible
  PCI: kirin: Add MODULE_* macros
  PCI: kirin: Allow building it as a module
  PCI: kirin: Add power_off support for Kirin 960 PHY
  PCI: kirin: fix poweroff sequence
  PCI: kirin: Allow removing the driver

 drivers/pci/controller/dwc/Kconfig      |   2 +-
 drivers/pci/controller/dwc/pcie-kirin.c | 644 +++++++++++++-----
 drivers/phy/hisilicon/Kconfig           |  10 +
 drivers/phy/hisilicon/Makefile          |   1 +
 drivers/phy/hisilicon/phy-hi3670-pcie.c | 845 ++++++++++++++++++++++++
 5 files changed, 1354 insertions(+), 148 deletions(-)
 create mode 100644 drivers/phy/hisilicon/phy-hi3670-pcie.c

-- 
2.31.1

[PATCH v12 02/11] 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>
---

To mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH v12 00/11] at: https://lore.kernel.org/all/cover.1632814194.git.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 v12 03/11] 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>
---

To mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH v12 00/11] at: https://lore.kernel.org/all/cover.1632814194.git.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 v12 04/11] 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>
---

To mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH v12 00/11] at: https://lore.kernel.org/all/cover.1632814194.git.mchehab+huawei@kernel.org/

 drivers/pci/controller/dwc/pcie-kirin.c | 49 +++++++++++++------------
 1 file changed, 26 insertions(+), 23 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index 31514a5d4bb4..0ea92a521e1c 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,25 +340,27 @@ static int hi3660_pcie_phy_init(struct platform_device *pdev,
  * 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 const struct regmap_config pcie_kirin_regmap_conf = {
+	.name = "kirin_pcie_apb",
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+};
 
 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 +370,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 +384,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 +450,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 +464,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 v12 05/11] PCI: kirin: Add support for bridge slot DT schema

[Mauro Carvalho Chehab]

On HiKey970, there's a PEX 8606 PCI bridge on its PHY with
6 lanes. Only 4 lanes are connected:

	lane 0 - connected to Kirin 970;
	lane 4 - M.2 slot;
	lane 5 - mini PCIe slot;
	lane 6 - in-board Ethernet controller.

Each lane has its own PERST# gpio pin, and needs a clock
request.

Add support to parse a DT schema containing the above data.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH v12 00/11] at: https://lore.kernel.org/all/cover.1632814194.git.mchehab+huawei@kernel.org/

 drivers/pci/controller/dwc/pcie-kirin.c | 264 +++++++++++++++++++++---
 1 file changed, 232 insertions(+), 32 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index 0ea92a521e1c..5c97e91adbb0 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -52,6 +52,19 @@
 #define PCIE_DEBOUNCE_PARAM	0xF0F400
 #define PCIE_OE_BYPASS		(0x3 << 28)
 
+/*
+ * Max number of connected PCI slots at an external PCI bridge
+ *
+ * This is used on HiKey 970, which has a PEX 8606 bridge with has
+ * 4 connected lanes (lane 0 upstream, and the other tree lanes,
+ * one connected to an in-board Ethernet adapter and the other two
+ * connected to M.2 and mini PCI slots.
+ *
+ * Each slot has a different clock source and uses a separate PERST#
+ * pin.
+ */
+#define MAX_PCI_SLOTS		3
+
 enum pcie_kirin_phy_type {
 	PCIE_KIRIN_INTERNAL_PHY,
 	PCIE_KIRIN_EXTERNAL_PHY
@@ -64,6 +77,19 @@ struct kirin_pcie {
 	struct regmap   *apb;
 	struct phy	*phy;
 	void		*phy_priv;	/* only for PCIE_KIRIN_INTERNAL_PHY */
+
+	/* DWC PERST# */
+	int		gpio_id_dwc_perst;
+
+	/* Per-slot PERST# */
+	int		num_slots;
+	int		gpio_id_reset[MAX_PCI_SLOTS];
+	const char	*reset_names[MAX_PCI_SLOTS];
+
+	/* Per-slot clkreq */
+	int		n_gpio_clkreq;
+	int		gpio_id_clkreq[MAX_PCI_SLOTS];
+	const char	*clkreq_names[MAX_PCI_SLOTS];
 };
 
 /*
@@ -87,7 +113,7 @@ struct kirin_pcie {
 #define CRGCTRL_PCIE_ASSERT_BIT		0x8c000000
 
 /* Time for delay */
-#define REF_2_PERST_MIN		20000
+#define REF_2_PERST_MIN		21000
 #define REF_2_PERST_MAX		25000
 #define PERST_2_ACCESS_MIN	10000
 #define PERST_2_ACCESS_MAX	12000
@@ -108,7 +134,6 @@ struct hi3660_pcie_phy {
 	struct clk	*phy_ref_clk;
 	struct clk	*aclk;
 	struct clk	*aux_clk;
-	int		gpio_id_reset;
 };
 
 /* Registers in PCIePHY */
@@ -171,16 +196,6 @@ static int hi3660_pcie_phy_get_resource(struct hi3660_pcie_phy *phy)
 	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;
 }
 
@@ -297,15 +312,7 @@ static int hi3660_pcie_phy_power_on(struct kirin_pcie *pcie)
 	if (ret)
 		goto disable_clks;
 
-	/* perst assert Endpoint */
-	if (!gpio_request(phy->gpio_id_reset, "pcie_perst")) {
-		usleep_range(REF_2_PERST_MIN, REF_2_PERST_MAX);
-		ret = gpio_direction_output(phy->gpio_id_reset, 1);
-		if (ret)
-			goto disable_clks;
-		usleep_range(PERST_2_ACCESS_MIN, PERST_2_ACCESS_MAX);
-		return 0;
-	}
+	return 0;
 
 disable_clks:
 	hi3660_pcie_phy_clk_ctrl(phy, false);
@@ -347,11 +354,98 @@ static const struct regmap_config pcie_kirin_regmap_conf = {
 	.reg_stride = 4,
 };
 
+static int kirin_pcie_get_gpio_enable(struct kirin_pcie *pcie,
+				      struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	char name[32];
+	int ret, i;
+
+	/* This is an optional property */
+	ret = of_gpio_named_count(np, "hisilicon,clken-gpios");
+	if (ret < 0)
+		return 0;
+
+	if (ret > MAX_PCI_SLOTS) {
+		dev_err(dev, "Too many GPIO clock requests!\n");
+		return -EINVAL;
+	}
+
+	pcie->n_gpio_clkreq = ret;
+
+	for (i = 0; i < pcie->n_gpio_clkreq; i++) {
+		pcie->gpio_id_clkreq[i] = of_get_named_gpio(dev->of_node,
+							    "hisilicon,clken-gpios", i);
+		if (pcie->gpio_id_clkreq[i] < 0)
+			return pcie->gpio_id_clkreq[i];
+
+		sprintf(name, "pcie_clkreq_%d", i);
+		pcie->clkreq_names[i] = devm_kstrdup_const(dev, name,
+							    GFP_KERNEL);
+		if (!pcie->clkreq_names[i])
+			return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static int kirin_pcie_parse_port(struct kirin_pcie *pcie,
+				 struct platform_device *pdev,
+				 struct device_node *node)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *parent, *child;
+	int ret, slot, i;
+	char name[32];
+
+	for_each_available_child_of_node(node, parent) {
+		for_each_available_child_of_node(parent, child) {
+			i = pcie->num_slots;
+
+			pcie->gpio_id_reset[i] = of_get_named_gpio(child,
+								"reset-gpios", 0);
+			if (pcie->gpio_id_reset[i] < 0)
+				continue;
+
+			pcie->num_slots++;
+			if (pcie->num_slots > MAX_PCI_SLOTS) {
+				dev_err(dev, "Too many PCI slots!\n");
+				return -EINVAL;
+			}
+
+			ret = of_pci_get_devfn(child);
+			if (ret < 0) {
+				dev_err(dev, "failed to parse devfn: %d\n", ret);
+				goto put_node;
+			}
+
+			slot = PCI_SLOT(ret);
+
+			sprintf(name, "pcie_perst_%d", slot);
+			pcie->reset_names[i] = devm_kstrdup_const(dev, name,
+								GFP_KERNEL);
+			if (!pcie->reset_names[i]) {
+				ret = -ENOMEM;
+				goto put_node;
+			}
+		}
+	}
+
+	return 0;
+
+put_node:
+	of_node_put(child);
+	return ret;
+}
+
 static long kirin_pcie_get_resource(struct kirin_pcie *kirin_pcie,
 				    struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node, *child;
 	void __iomem *apb_base;
+	int ret;
 
 	apb_base = devm_platform_ioremap_resource_byname(pdev, "apb");
 	if (IS_ERR(apb_base))
@@ -362,7 +456,32 @@ static long kirin_pcie_get_resource(struct kirin_pcie *kirin_pcie,
 	if (IS_ERR(kirin_pcie->apb))
 		return PTR_ERR(kirin_pcie->apb);
 
+	/* pcie internal PERST# gpio */
+	kirin_pcie->gpio_id_dwc_perst = of_get_named_gpio(dev->of_node,
+							  "reset-gpios", 0);
+	if (kirin_pcie->gpio_id_dwc_perst == -EPROBE_DEFER) {
+		return -EPROBE_DEFER;
+	} else if (!gpio_is_valid(kirin_pcie->gpio_id_dwc_perst)) {
+		dev_err(dev, "unable to get a valid gpio pin\n");
+		return -ENODEV;
+	}
+
+	ret = kirin_pcie_get_gpio_enable(kirin_pcie, pdev);
+	if (ret)
+		return ret;
+
+	/* Parse OF children */
+	for_each_available_child_of_node(node, child) {
+		ret = kirin_pcie_parse_port(kirin_pcie, pdev, child);
+		if (ret)
+			goto put_node;
+	}
+
 	return 0;
+
+put_node:
+	of_node_put(child);
+	return ret;
 }
 
 static void kirin_pcie_sideband_dbi_w_mode(struct kirin_pcie *kirin_pcie,
@@ -419,9 +538,33 @@ static int kirin_pcie_wr_own_conf(struct pci_bus *bus, unsigned int devfn,
 	return PCIBIOS_SUCCESSFUL;
 }
 
+static int kirin_pcie_add_bus(struct pci_bus *bus)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(bus->sysdata);
+	struct kirin_pcie *kirin_pcie = to_kirin_pcie(pci);
+	int i, ret;
+
+	if (!kirin_pcie->num_slots)
+		return 0;
+
+	/* Send PERST# to each slot */
+	for (i = 0; i < kirin_pcie->num_slots; i++) {
+		ret = gpio_direction_output(kirin_pcie->gpio_id_reset[i], 1);
+		if (ret) {
+			dev_err(pci->dev, "PERST# %s error: %d\n",
+				kirin_pcie->reset_names[i], ret);
+		}
+	}
+	usleep_range(PERST_2_ACCESS_MIN, PERST_2_ACCESS_MAX);
+
+	return 0;
+}
+
+
 static struct pci_ops kirin_pci_ops = {
 	.read = kirin_pcie_rd_own_conf,
 	.write = kirin_pcie_wr_own_conf,
+	.add_bus = kirin_pcie_add_bus,
 };
 
 static u32 kirin_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base,
@@ -477,6 +620,44 @@ static int kirin_pcie_host_init(struct pcie_port *pp)
 	return 0;
 }
 
+static int kirin_pcie_gpio_request(struct kirin_pcie *kirin_pcie,
+				   struct device *dev)
+{
+	int ret, i;
+
+	for (i = 0; i < kirin_pcie->num_slots; i++) {
+		if (!gpio_is_valid(kirin_pcie->gpio_id_reset[i])) {
+			dev_err(dev, "unable to get a valid %s gpio\n",
+				kirin_pcie->reset_names[i]);
+			return -ENODEV;
+		}
+
+		ret = devm_gpio_request(dev, kirin_pcie->gpio_id_reset[i],
+					kirin_pcie->reset_names[i]);
+		if (ret)
+			return ret;
+	}
+
+	for (i = 0; i < kirin_pcie->n_gpio_clkreq; i++) {
+		if (!gpio_is_valid(kirin_pcie->gpio_id_clkreq[i])) {
+			dev_err(dev, "unable to get a valid %s gpio\n",
+				kirin_pcie->clkreq_names[i]);
+			return -ENODEV;
+		}
+
+		ret = devm_gpio_request(dev, kirin_pcie->gpio_id_clkreq[i],
+					kirin_pcie->clkreq_names[i]);
+		if (ret)
+			return ret;
+
+		ret = gpio_direction_output(kirin_pcie->gpio_id_clkreq[i], 0);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 static const struct dw_pcie_ops kirin_dw_pcie_ops = {
 	.read_dbi = kirin_pcie_read_dbi,
 	.write_dbi = kirin_pcie_write_dbi,
@@ -499,24 +680,43 @@ static int kirin_pcie_power_on(struct platform_device *pdev,
 		if (ret)
 			return ret;
 
-		return hi3660_pcie_phy_power_on(kirin_pcie);
+		ret = hi3660_pcie_phy_power_on(kirin_pcie);
+		if (ret)
+			return ret;
+	} else {
+		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 = kirin_pcie_gpio_request(kirin_pcie, dev);
+		if (ret)
+			return ret;
+
+		ret = phy_init(kirin_pcie->phy);
+		if (ret)
+			goto err;
+
+		ret = phy_power_on(kirin_pcie->phy);
+		if (ret)
+			goto err;
 	}
 
-	kirin_pcie->phy = devm_of_phy_get(dev, dev->of_node, NULL);
-	if (IS_ERR(kirin_pcie->phy))
-		return PTR_ERR(kirin_pcie->phy);
+	/* perst assert Endpoint */
+	usleep_range(REF_2_PERST_MIN, REF_2_PERST_MAX);
 
-	ret = phy_init(kirin_pcie->phy);
-	if (ret)
-		goto err;
+	if (!gpio_request(kirin_pcie->gpio_id_dwc_perst, "pcie_perst_bridge")) {
+		ret = gpio_direction_output(kirin_pcie->gpio_id_dwc_perst, 1);
+		if (ret)
+			goto err;
+	}
 
-	ret = phy_power_on(kirin_pcie->phy);
-	if (ret)
-		goto err;
+	usleep_range(PERST_2_ACCESS_MIN, PERST_2_ACCESS_MAX);
 
 	return 0;
 err:
-	phy_exit(kirin_pcie->phy);
+	if (kirin_pcie->type != PCIE_KIRIN_INTERNAL_PHY)
+		phy_exit(kirin_pcie->phy);
+
 	return ret;
 }
 
-- 
2.31.1

[PATCH v12 06/11] PCI: kirin: Add Kirin 970 compatible

[Mauro Carvalho Chehab]

Now that everything is in place, add a compatible for Kirin 970.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH v12 00/11] at: https://lore.kernel.org/all/cover.1632814194.git.mchehab+huawei@kernel.org/

 drivers/pci/controller/dwc/pcie-kirin.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index 5c97e91adbb0..4f4c86b92353 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -738,6 +738,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
+	},
 	{},
 };
 
-- 
2.31.1

[PATCH v12 07/11] 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>
---

To mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH v12 00/11] at: https://lore.kernel.org/all/cover.1632814194.git.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 4f4c86b92353..d5c29a266756 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -804,3 +804,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 v12 08/11] 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>
---

To mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH v12 00/11] at: https://lore.kernel.org/all/cover.1632814194.git.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 76c0a63a3f64..a07c08d714c9 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -266,7 +266,7 @@ config PCIE_KEEMBAY_EP
 
 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

[PATCH v12 09/11] PCI: kirin: Add power_off support for Kirin 960 PHY

[Mauro Carvalho Chehab]

In order to prepare for module unload, add a power_off method
for HiKey 960.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH v12 00/11] at: https://lore.kernel.org/all/cover.1632814194.git.mchehab+huawei@kernel.org/

 drivers/pci/controller/dwc/pcie-kirin.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index d5c29a266756..b17a194cf78d 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -343,6 +343,18 @@ static int hi3660_pcie_phy_init(struct platform_device *pdev,
 	return hi3660_pcie_phy_get_resource(phy);
 }
 
+static int hi3660_pcie_phy_power_off(struct kirin_pcie *pcie)
+{
+	struct hi3660_pcie_phy *phy = pcie->phy_priv;
+
+	/* Drop power supply for Host */
+	regmap_write(phy->sysctrl, SCTRL_PCIE_CMOS_OFFSET, 0x00);
+
+	hi3660_pcie_phy_clk_ctrl(phy, false);
+
+	return 0;
+}
+
 /*
  * The non-PHY part starts here
  */
@@ -560,7 +572,6 @@ static int kirin_pcie_add_bus(struct pci_bus *bus)
 	return 0;
 }
 
-
 static struct pci_ops kirin_pci_ops = {
 	.read = kirin_pcie_rd_own_conf,
 	.write = kirin_pcie_wr_own_conf,
@@ -714,8 +725,12 @@ static int kirin_pcie_power_on(struct platform_device *pdev,
 
 	return 0;
 err:
-	if (kirin_pcie->type != PCIE_KIRIN_INTERNAL_PHY)
+	if (kirin_pcie->type == PCIE_KIRIN_INTERNAL_PHY) {
+		hi3660_pcie_phy_power_off(kirin_pcie);
+	} else {
+		phy_power_off(kirin_pcie->phy);
 		phy_exit(kirin_pcie->phy);
+	}
 
 	return ret;
 }
@@ -725,7 +740,7 @@ 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;
+		return hi3660_pcie_phy_power_off(kirin_pcie);
 
 	phy_power_off(kirin_pcie->phy);
 	phy_exit(kirin_pcie->phy);
-- 
2.31.1

[PATCH v12 10/11] PCI: kirin: fix poweroff sequence

[Mauro Carvalho Chehab]

This driver currently doesn't call dw_pcie_host_deinit()
at the .remove() callback. This can cause an OOPS if the driver
is unbound.

While here, add a poweroff function, in order to abstract
between the internal and external PHY logic.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH v12 00/11] at: https://lore.kernel.org/all/cover.1632814194.git.mchehab+huawei@kernel.org/

 drivers/pci/controller/dwc/pcie-kirin.c | 30 ++++++++++++++++---------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index b17a194cf78d..ffc63d12f8ed 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -680,6 +680,23 @@ static const struct dw_pcie_host_ops kirin_pcie_host_ops = {
 	.host_init = kirin_pcie_host_init,
 };
 
+static int kirin_pcie_power_off(struct kirin_pcie *kirin_pcie)
+{
+	int i;
+
+	if (kirin_pcie->type == PCIE_KIRIN_INTERNAL_PHY)
+		return hi3660_pcie_phy_power_off(kirin_pcie);
+
+	for (i = 0; i < kirin_pcie->n_gpio_clkreq; i++) {
+		gpio_direction_output(kirin_pcie->gpio_id_clkreq[i], 1);
+	}
+
+	phy_power_off(kirin_pcie->phy);
+	phy_exit(kirin_pcie->phy);
+
+	return 0;
+}
+
 static int kirin_pcie_power_on(struct platform_device *pdev,
 			       struct kirin_pcie *kirin_pcie)
 {
@@ -725,12 +742,7 @@ static int kirin_pcie_power_on(struct platform_device *pdev,
 
 	return 0;
 err:
-	if (kirin_pcie->type == PCIE_KIRIN_INTERNAL_PHY) {
-		hi3660_pcie_phy_power_off(kirin_pcie);
-	} else {
-		phy_power_off(kirin_pcie->phy);
-		phy_exit(kirin_pcie->phy);
-	}
+	kirin_pcie_power_off(kirin_pcie);
 
 	return ret;
 }
@@ -739,11 +751,9 @@ 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 hi3660_pcie_phy_power_off(kirin_pcie);
+	dw_pcie_host_deinit(&kirin_pcie->pci->pp);
 
-	phy_power_off(kirin_pcie->phy);
-	phy_exit(kirin_pcie->phy);
+	kirin_pcie_power_off(kirin_pcie);
 
 	return 0;
 }
-- 
2.31.1

[PATCH v12 11/11] PCI: kirin: Allow removing the driver

[Mauro Carvalho Chehab]

Now that everything is in place at the poweroff sequence,
this driver can use module_platform_driver(), which allows
it to be removed.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH v12 00/11] at: https://lore.kernel.org/all/cover.1632814194.git.mchehab+huawei@kernel.org/

 drivers/pci/controller/dwc/pcie-kirin.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index ffc63d12f8ed..efab2af093b6 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -828,7 +828,7 @@ static struct platform_driver kirin_pcie_driver = {
 		.suppress_bind_attrs	= true,
 	},
 };
-builtin_platform_driver(kirin_pcie_driver);
+module_platform_driver(kirin_pcie_driver);
 
 MODULE_DEVICE_TABLE(of, kirin_pcie_match);
 MODULE_DESCRIPTION("PCIe host controller driver for Kirin Phone SoCs");
-- 
2.31.1

Re: [PATCH v12 00/11] Add support for Hikey 970 PCIe

[Mauro Carvalho Chehab]

Hi Bjorn,

Em Tue, 28 Sep 2021 09:34:10 +0200
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:

> The pcie-kirin PCIe driver contains internally a PHY interface for
> Kirin 960, but it misses support for Kirin 970.
> 
> Patch1 contains a PHY for Kirin 970 PCIe.
> 
> The remaining patches add support for Kirin 970 at the pcie-kirin driver, and
> add the needed logic to compile it as module and to allow to dynamically
> remove the driver in runtime.
> 
> Tested on HiKey970:
> 
>   # lspci -D -PP
>   0000:00:00.0 PCI bridge: Huawei Technologies Co., Ltd. Device 3670 (rev 01)
>   0000:00:00.0/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)
>   0000:00:00.0/01:00.0/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)
>   0000:00:00.0/01:00.0/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)
>   0000:00:00.0/01:00.0/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)
>   0000:00:00.0/01:00.0/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)
>   0000:00:00.0/01:00.0/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)
>   0000:00:00.0/01:00.0/02:01.0/03:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd Device a809
>   0000:00:00.0/01:00.0/02:07.0/06:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 07)
> 
> Tested on HiKey960:
> 
>   # lspci -D 
>   0000:00:00.0 PCI bridge: Huawei Technologies Co., Ltd. Device 3660 (rev 01)
> 
> ---
> 
> v12:
>   - Change a comment at patch 1 to not use c99 style.
> 
> v11:
>   - patch 5 changed to use the right PCIe topology
>   - all other patches are identical to v10.
> 
> v10:
>   - patch 1: dropped magic numbers from PHY driver
>   - patch 5: allow pcie child nodes without reset-gpios
>   - all other patches are identical to v9.
> 
> v9:
>   - Did some cleanups at patches 1 and 5
> 

As the DT changes needed by HiKey 970 PCIe support are already upstream:

	commit cfcf126fc6795e843d090d98754391ece55e8b0c
	Author:     Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
	AuthorDate: Wed Aug 4 09:18:56 2021 +0200
	Commit:     Rob Herring <robh@kernel.org>
	CommitDate: Mon Aug 16 16:00:52 2021 -0500

	    dt-bindings: PCI: kirin: Add support for Kirin970
	    
	    Add a new compatible, plus the new bindings needed by
	    HiKey970 board.
    
	    Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
	    Link: https://lore.kernel.org/r/875a4571e253040d3885ee1f37467b0bade7361b.1628061310.git.mchehab+huawei@kernel.org
	    Signed-off-by: Rob Herring <robh@kernel.org>

> 
> Mauro Carvalho Chehab (11):
>   phy: HiSilicon: Add driver for Kirin 970 PCIe PHY

And the PHY patch was already accepted and merged at today's
linux-next:

	commit 73075011ffff876de8516a1e583dc41869293da9
	Author:     Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
	AuthorDate: Tue Sep 28 09:34:11 2021 +0200
	Commit:     Vinod Koul <vkoul@kernel.org>
	CommitDate: Fri Oct 1 13:42:18 2021 +0530

	    phy: HiSilicon: Add driver for Kirin 970 PCIe PHY
    
	    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>
	    Link: https://lore.kernel.org/r/b7a4ff41b57d861b003f1a00cae81f3d226fbe18.1632814194.git.mchehab+huawei@kernel.org
	    Signed-off-by: Vinod Koul <vkoul@kernel.org>

>   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 support for bridge slot DT schema
>   PCI: kirin: Add Kirin 970 compatible
>   PCI: kirin: Add MODULE_* macros
>   PCI: kirin: Allow building it as a module
>   PCI: kirin: Add power_off support for Kirin 960 PHY
>   PCI: kirin: fix poweroff sequence
>   PCI: kirin: Allow removing the driver

I guess everything is already satisfying the review feedbacks.
If so, could you please merge the PCI ones?

> 
>  drivers/pci/controller/dwc/Kconfig      |   2 +-
>  drivers/pci/controller/dwc/pcie-kirin.c | 644 +++++++++++++-----
>  drivers/phy/hisilicon/Kconfig           |  10 +
>  drivers/phy/hisilicon/Makefile          |   1 +
>  drivers/phy/hisilicon/phy-hi3670-pcie.c | 845 ++++++++++++++++++++++++
>  5 files changed, 1354 insertions(+), 148 deletions(-)
>  create mode 100644 drivers/phy/hisilicon/phy-hi3670-pcie.c

Thanks,
Mauro

Re: [PATCH v12 00/11] Add support for Hikey 970 PCIe

[Bjorn Helgaas]

[+cc Lorenzo]

On Tue, Oct 05, 2021 at 11:24:48AM +0200, Mauro Carvalho Chehab wrote:
> Hi Bjorn,
> 
> Em Tue, 28 Sep 2021 09:34:10 +0200
> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:

> >   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 support for bridge slot DT schema
> >   PCI: kirin: Add Kirin 970 compatible
> >   PCI: kirin: Add MODULE_* macros
> >   PCI: kirin: Allow building it as a module
> >   PCI: kirin: Add power_off support for Kirin 960 PHY
> >   PCI: kirin: fix poweroff sequence
> >   PCI: kirin: Allow removing the driver
> 
> I guess everything is already satisfying the review feedbacks.
> If so, could you please merge the PCI ones?

Lorenzo takes care of the native host bridge drivers, so I'm sure this
is on his list.  I added him to cc: in case not.

Re: [PATCH v12 03/11] PCI: kirin: Add support for a PHY layer

[Pali Rohár]

Hello!

On Tuesday 28 September 2021 09:34:13 Mauro Carvalho Chehab wrote:
> 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.

I have not looked deeply at it. But is not it really possible to declare
PHY node in DTS file with backward compatible manner? Or cannot Rob help
with it (maybe there was similar issue in past with other driver)?

I was fixing something similar, address space defined in DTS was used by
two HW blocks: clock and UART. And I was able to make both DTS file and
driver backward compatible.

Re: [PATCH v12 03/11] PCI: kirin: Add support for a PHY layer

[Mauro Carvalho Chehab]

Hi Pali,

Em Tue, 5 Oct 2021 22:31:48 +0200
Pali Rohár <pali@kernel.org> escreveu:

> Hello!
> 
> On Tuesday 28 September 2021 09:34:13 Mauro Carvalho Chehab wrote:
> > 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.  
> 
> I have not looked deeply at it. But is not it really possible to declare
> PHY node in DTS file with backward compatible manner? Or cannot Rob help
> with it (maybe there was similar issue in past with other driver)?

It would be possible to split the Kirin 960 PHY into drivers/phy. It is
also possible to hack the phy driver to search for the PHY-specific data
inside the PCI compatible string (I did wrote some patches doing that
and sent as a RFC several months ago), but the problem is that the
PHY driver won't be probed without adding a new compatible inside the
DT schema. By doing that, the schema will be incompatible.

In any case, the patches on this series split all PHY-specific code
inside the driver on a separate part of the source code. Moving it
to a new driver would be easy once someone comes with a solution
to add some new method at the PHY layer that would allow to load a
new module without having a compatible for it.

> I was fixing something similar, address space defined in DTS was used by
> two HW blocks: clock and UART. And I was able to make both DTS file and
> driver backward compatible.

Thanks,
Mauro

Re: [PATCH v12 00/11] Add support for Hikey 970 PCIe

[Lorenzo Pieralisi]

On Tue, Oct 05, 2021 at 01:23:21PM -0500, Bjorn Helgaas wrote:
> [+cc Lorenzo]
> 
> On Tue, Oct 05, 2021 at 11:24:48AM +0200, Mauro Carvalho Chehab wrote:
> > Hi Bjorn,
> > 
> > Em Tue, 28 Sep 2021 09:34:10 +0200
> > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
> 
> > >   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 support for bridge slot DT schema
> > >   PCI: kirin: Add Kirin 970 compatible
> > >   PCI: kirin: Add MODULE_* macros
> > >   PCI: kirin: Allow building it as a module
> > >   PCI: kirin: Add power_off support for Kirin 960 PHY
> > >   PCI: kirin: fix poweroff sequence
> > >   PCI: kirin: Allow removing the driver
> > 
> > I guess everything is already satisfying the review feedbacks.
> > If so, could you please merge the PCI ones?
> 
> Lorenzo takes care of the native host bridge drivers, so I'm sure this
> is on his list.  I added him to cc: in case not.

Ideally I'd like to see these patches ACKed/Review-ed by the kirin
maintainers - that's what I was waiting for and that's what they
are there for.

Having said that, I will keep an eye on this series so that we
can hopefully queue it for v5.16.

Lorenzo

Re: [PATCH v12 00/11] Add support for Hikey 970 PCIe

[Mauro Carvalho Chehab]

Hi Lorenzo,

Em Thu, 7 Oct 2021 15:41:03 +0100
Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> escreveu:

> On Tue, Oct 05, 2021 at 01:23:21PM -0500, Bjorn Helgaas wrote:
> > [+cc Lorenzo]
> > 
> > On Tue, Oct 05, 2021 at 11:24:48AM +0200, Mauro Carvalho Chehab wrote:  
> > > Hi Bjorn,
> > > 
> > > Em Tue, 28 Sep 2021 09:34:10 +0200
> > > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:  
> >   
> > > >   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 support for bridge slot DT schema
> > > >   PCI: kirin: Add Kirin 970 compatible
> > > >   PCI: kirin: Add MODULE_* macros
> > > >   PCI: kirin: Allow building it as a module
> > > >   PCI: kirin: Add power_off support for Kirin 960 PHY
> > > >   PCI: kirin: fix poweroff sequence
> > > >   PCI: kirin: Allow removing the driver  
> > > 
> > > I guess everything is already satisfying the review feedbacks.
> > > If so, could you please merge the PCI ones?  
> > 
> > Lorenzo takes care of the native host bridge drivers, so I'm sure this
> > is on his list.  I added him to cc: in case not.  
> 
> Ideally I'd like to see these patches ACKed/Review-ed by the kirin
> maintainers - that's what I was waiting for and that's what they
> are there for.
> 
> Having said that, I will keep an eye on this series so that we
> can hopefully queue it for v5.16.

Not sure if you received the e-mail from Xiaowei with his ack.

At least here, I only received on my internal e-mail (perhaps because
the original e-mail was base64-encoded with gb2312 charset). 

So, let me forward his answer to you, c/c the mailing lists.

Thanks,
Mauro

-------- Forwarded Message --------
From: Songxiaowei (Kirin_DRV) <songxiaowei@hisilicon.com>
To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, Bjorn Helgaas <helgaas@kernel.org>
CC: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>, Bjorn Helgaas <bhelgaas@google.com>, Linuxarm <linuxarm@huawei.com>, Mauro Carvalho Chehab <mauro.chehab@huawei.com>, Krzysztof Wilczyński <kw@linux.com>, Wangbinghui (Biggio, Kirin_DRV) <wangbinghui@hisilicon.com>, Rob Herring <robh@kernel.org>, linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>, linux-pci@vger.kernel.org <linux-pci@vger.kernel.org>, linux-phy@lists.infradead.org <linux-phy@lists.infradead.org>, Kongfei <kongfei@hisilicon.com>
Subject: Re: [PATCH v12 00/11] Add support for Hikey 970 PCIe
Date: Fri, 8 Oct 2021 11:45:06 +0100
Message-ID: <e718dc06633e4f87a6b6e1626e8c098e@hisilicon.com>

Hi Bjorn,

ACKed, it seems ok to me and Binghui.

Thanks a lot.

B. R.

-----邮件原件-----
发件人: Lorenzo Pieralisi [mailto:lorenzo.pieralisi@arm.com]
发送时间: 2021年10月7日 22:41
收件人: Bjorn Helgaas <helgaas@kernel.org>
抄送: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>; Bjorn Helgaas <bhelgaas@google.com>; Linuxarm <linuxarm@huawei.com>; Mauro Carvalho Chehab <mauro.chehab@huawei.com>; Krzysztof Wilczyński <kw@linux.com>; Wangbinghui (Biggio, Kirin_DRV) <wangbinghui@hisilicon.com>; Rob Herring <robh@kernel.org>; Songxiaowei (Kirin_DRV) <songxiaowei@hisilicon.com>; linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org; linux-phy@lists.infradead.org
主题: Re: [PATCH v12 00/11] Add support for Hikey 970 PCIe

On Tue, Oct 05, 2021 at 01:23:21PM -0500, Bjorn Helgaas wrote:
> [+cc Lorenzo]
> 
> On Tue, Oct 05, 2021 at 11:24:48AM +0200, Mauro Carvalho Chehab wrote:  
> > Hi Bjorn,
> > 
> > Em Tue, 28 Sep 2021 09:34:10 +0200
> > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:  
>   
> > >   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 support for bridge slot DT schema
> > >   PCI: kirin: Add Kirin 970 compatible
> > >   PCI: kirin: Add MODULE_* macros
> > >   PCI: kirin: Allow building it as a module
> > >   PCI: kirin: Add power_off support for Kirin 960 PHY
> > >   PCI: kirin: fix poweroff sequence
> > >   PCI: kirin: Allow removing the driver  
> > 
> > I guess everything is already satisfying the review feedbacks.
> > If so, could you please merge the PCI ones?  
> 
> Lorenzo takes care of the native host bridge drivers, so I'm sure this 
> is on his list.  I added him to cc: in case not.  

Ideally I'd like to see these patches ACKed/Review-ed by the kirin maintainers - that's what I was waiting for and that's what they are there for.

Having said that, I will keep an eye on this series so that we can hopefully queue it for v5.16.

Lorenzo

Re: [PATCH v12 00/11] Add support for Hikey 970 PCIe

[Lorenzo Pieralisi]

On Fri, Oct 08, 2021 at 12:55:21PM +0200, Mauro Carvalho Chehab wrote:
> Hi Lorenzo,
> 
> Em Thu, 7 Oct 2021 15:41:03 +0100
> Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> escreveu:
> 
> > On Tue, Oct 05, 2021 at 01:23:21PM -0500, Bjorn Helgaas wrote:
> > > [+cc Lorenzo]
> > > 
> > > On Tue, Oct 05, 2021 at 11:24:48AM +0200, Mauro Carvalho Chehab wrote:  
> > > > Hi Bjorn,
> > > > 
> > > > Em Tue, 28 Sep 2021 09:34:10 +0200
> > > > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:  
> > >   
> > > > >   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 support for bridge slot DT schema
> > > > >   PCI: kirin: Add Kirin 970 compatible
> > > > >   PCI: kirin: Add MODULE_* macros
> > > > >   PCI: kirin: Allow building it as a module
> > > > >   PCI: kirin: Add power_off support for Kirin 960 PHY
> > > > >   PCI: kirin: fix poweroff sequence
> > > > >   PCI: kirin: Allow removing the driver  
> > > > 
> > > > I guess everything is already satisfying the review feedbacks.
> > > > If so, could you please merge the PCI ones?  
> > > 
> > > Lorenzo takes care of the native host bridge drivers, so I'm sure this
> > > is on his list.  I added him to cc: in case not.  
> > 
> > Ideally I'd like to see these patches ACKed/Review-ed by the kirin
> > maintainers - that's what I was waiting for and that's what they
> > are there for.
> > 
> > Having said that, I will keep an eye on this series so that we
> > can hopefully queue it for v5.16.
> 
> Not sure if you received the e-mail from Xiaowei with his ack.

I have not (and it did not make it to linux-pci either).

> At least here, I only received on my internal e-mail (perhaps because
> the original e-mail was base64-encoded with gb2312 charset). 
> 
> So, let me forward his answer to you, c/c the mailing lists.

Patches should be acked with tags that tooling recognize, this
would help me.

> Thanks,
> Mauro
> 
> -------- Forwarded Message --------
> From: Songxiaowei (Kirin_DRV) <songxiaowei@hisilicon.com>
> To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, Bjorn Helgaas <helgaas@kernel.org>
> CC: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>, Bjorn Helgaas <bhelgaas@google.com>, Linuxarm <linuxarm@huawei.com>, Mauro Carvalho Chehab <mauro.chehab@huawei.com>, Krzysztof Wilczyński <kw@linux.com>, Wangbinghui (Biggio, Kirin_DRV) <wangbinghui@hisilicon.com>, Rob Herring <robh@kernel.org>, linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>, linux-pci@vger.kernel.org <linux-pci@vger.kernel.org>, linux-phy@lists.infradead.org <linux-phy@lists.infradead.org>, Kongfei <kongfei@hisilicon.com>
> Subject: Re: [PATCH v12 00/11] Add support for Hikey 970 PCIe
> Date: Fri, 8 Oct 2021 11:45:06 +0100
> Message-ID: <e718dc06633e4f87a6b6e1626e8c098e@hisilicon.com>
> 
> Hi Bjorn,
> 
> ACKed, it seems ok to me and Binghui.

For Xiaowei:

https://www.kernel.org/doc/html/latest/process/submitting-patches.html

and your email must make it to the mailing list; if it does not
it does not exist as far as I am concerned. I will apply the
ACK manually for this time but let's keep this in mind please.

Thanks,
Lorenzo
> 
> Thanks a lot.
> 
> B. R.
> 
> -----邮件原件-----
> 发件人: Lorenzo Pieralisi [mailto:lorenzo.pieralisi@arm.com]
> 发送时间: 2021年10月7日 22:41
> 收件人: Bjorn Helgaas <helgaas@kernel.org>
> 抄送: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>; Bjorn Helgaas <bhelgaas@google.com>; Linuxarm <linuxarm@huawei.com>; Mauro Carvalho Chehab <mauro.chehab@huawei.com>; Krzysztof Wilczyński <kw@linux.com>; Wangbinghui (Biggio, Kirin_DRV) <wangbinghui@hisilicon.com>; Rob Herring <robh@kernel.org>; Songxiaowei (Kirin_DRV) <songxiaowei@hisilicon.com>; linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org; linux-phy@lists.infradead.org
> 主题: Re: [PATCH v12 00/11] Add support for Hikey 970 PCIe
> 
> On Tue, Oct 05, 2021 at 01:23:21PM -0500, Bjorn Helgaas wrote:
> > [+cc Lorenzo]
> > 
> > On Tue, Oct 05, 2021 at 11:24:48AM +0200, Mauro Carvalho Chehab wrote:  
> > > Hi Bjorn,
> > > 
> > > Em Tue, 28 Sep 2021 09:34:10 +0200
> > > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:  
> >   
> > > >   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 support for bridge slot DT schema
> > > >   PCI: kirin: Add Kirin 970 compatible
> > > >   PCI: kirin: Add MODULE_* macros
> > > >   PCI: kirin: Allow building it as a module
> > > >   PCI: kirin: Add power_off support for Kirin 960 PHY
> > > >   PCI: kirin: fix poweroff sequence
> > > >   PCI: kirin: Allow removing the driver  
> > > 
> > > I guess everything is already satisfying the review feedbacks.
> > > If so, could you please merge the PCI ones?  
> > 
> > Lorenzo takes care of the native host bridge drivers, so I'm sure this 
> > is on his list.  I added him to cc: in case not.  
> 
> Ideally I'd like to see these patches ACKed/Review-ed by the kirin maintainers - that's what I was waiting for and that's what they are there for.
> 
> Having said that, I will keep an eye on this series so that we can hopefully queue it for v5.16.
> 
> Lorenzo
> 
> 

Re: [PATCH v12 00/11] Add support for Hikey 970 PCIe

[Lorenzo Pieralisi]

On Sat, Oct 09, 2021 at 01:44:24AM +0000, Songxiaowei (Kirin_DRV) wrote:
> Ack-by Xiaowei Song.

First of all, you should not top-post:

https://kernelnewbies.org/PatchCulture

Second the format does not match the guidelines:

https://www.kernel.org/doc/html/latest/process/submitting-patches.html

So let's retry to send your ACK a third time please.

Thanks,
Lorenzo

> 
> > Hi Lorenzo,
> > 
> > Em Thu, 7 Oct 2021 15:41:03 +0100
> > Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> escreveu:
> > 
> > > On Tue, Oct 05, 2021 at 01:23:21PM -0500, Bjorn Helgaas wrote:
> > > > [+cc Lorenzo]
> > > > 
> > > > On Tue, Oct 05, 2021 at 11:24:48AM +0200, Mauro Carvalho Chehab wrote:  
> > > > > Hi Bjorn,
> > > > > 
> > > > > Em Tue, 28 Sep 2021 09:34:10 +0200
> > > > > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:  
> > > >   
> > > > > >   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 support for bridge slot DT schema
> > > > > >   PCI: kirin: Add Kirin 970 compatible
> > > > > >   PCI: kirin: Add MODULE_* macros
> > > > > >   PCI: kirin: Allow building it as a module
> > > > > >   PCI: kirin: Add power_off support for Kirin 960 PHY
> > > > > >   PCI: kirin: fix poweroff sequence
> > > > > >   PCI: kirin: Allow removing the driver  
> > > > > 
> > > > > I guess everything is already satisfying the review feedbacks.
> > > > > If so, could you please merge the PCI ones?  
> > > > 
> > > > Lorenzo takes care of the native host bridge drivers, so I'm sure this
> > > > is on his list.  I added him to cc: in case not.  
> > > 
> > > Ideally I'd like to see these patches ACKed/Review-ed by the kirin
> > > maintainers - that's what I was waiting for and that's what they
> > > are there for.
> > > 
> > > Having said that, I will keep an eye on this series so that we
> > > can hopefully queue it for v5.16.
> > 
> > Not sure if you received the e-mail from Xiaowei with his ack.
> 
> I have not (and it did not make it to linux-pci either).
> 
> > At least here, I only received on my internal e-mail (perhaps because
> > the original e-mail was base64-encoded with gb2312 charset). 
> > 
> > So, let me forward his answer to you, c/c the mailing lists.
> 
> Patches should be acked with tags that tooling recognize, this
> would help me.
> 
> > Thanks,
> > Mauro
> > 
> 
> Thanks.

Re: [PATCH v12 00/11] Add support for Hikey 970 PCIe

[Songxiaowei (Kirin_DRV)]

Ack-by Xiaowei Song.

> Hi Lorenzo,
> 
> Em Thu, 7 Oct 2021 15:41:03 +0100
> Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> escreveu:
> 
> > On Tue, Oct 05, 2021 at 01:23:21PM -0500, Bjorn Helgaas wrote:
> > > [+cc Lorenzo]
> > > 
> > > On Tue, Oct 05, 2021 at 11:24:48AM +0200, Mauro Carvalho Chehab wrote:  
> > > > Hi Bjorn,
> > > > 
> > > > Em Tue, 28 Sep 2021 09:34:10 +0200
> > > > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:  
> > >   
> > > > >   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 support for bridge slot DT schema
> > > > >   PCI: kirin: Add Kirin 970 compatible
> > > > >   PCI: kirin: Add MODULE_* macros
> > > > >   PCI: kirin: Allow building it as a module
> > > > >   PCI: kirin: Add power_off support for Kirin 960 PHY
> > > > >   PCI: kirin: fix poweroff sequence
> > > > >   PCI: kirin: Allow removing the driver  
> > > > 
> > > > I guess everything is already satisfying the review feedbacks.
> > > > If so, could you please merge the PCI ones?  
> > > 
> > > Lorenzo takes care of the native host bridge drivers, so I'm sure this
> > > is on his list.  I added him to cc: in case not.  
> > 
> > Ideally I'd like to see these patches ACKed/Review-ed by the kirin
> > maintainers - that's what I was waiting for and that's what they
> > are there for.
> > 
> > Having said that, I will keep an eye on this series so that we
> > can hopefully queue it for v5.16.
> 
> Not sure if you received the e-mail from Xiaowei with his ack.

I have not (and it did not make it to linux-pci either).

> At least here, I only received on my internal e-mail (perhaps because
> the original e-mail was base64-encoded with gb2312 charset). 
> 
> So, let me forward his answer to you, c/c the mailing lists.

Patches should be acked with tags that tooling recognize, this
would help me.

> Thanks,
> Mauro
> 

Thanks.