[PATCH v5 0/9] PCI: mediatek: fixup find_port, enable_msi and add pm, module support

[PATCH v5 0/9] PCI: mediatek: fixup find_port, enable_msi and add pm, module support

[honghui.zhang]

From: Honghui Zhang <honghui.zhang@mediatek.com>

This patchset includes misc patchs:

The patch 1 fixup the mtk_pcie_find_port logic which will cause system
could not touch the EP's configuration space that connected to PCIe slot 1.

The patch 2 fixup the class type for MT7622.
The patch 6 fixup the enable msi logic, the operation to enable msi
should be after system clock is enabled. Call mtk_pcie_enable_msi in
mtk_pcie_startup_port_v2 since the clock was all enabled at that time.

The patch 7 was rebased and refactor of the v4 patch[1], changes are:
 -Add PM support for MT7622.
 -Using mtk_pcie_enable_port to re-establish the link when resumed.
 -Rebased on this patchset.

The patch 9 add loadable kernel module support.

[1] https://patchwork.kernel.org/patch/10479079

Change since v4:
 - Add patch 2 to fixup class type for MT7622.
 - Add patch 3 to remove the redundant dev->pm_domain check
 - Add patch 4 to covert to use pci_host_probe
 - Add patch 5 to re-arrange the function define, this is a prepare patch for
   fixup the enable_msi logic, no functional changed have been made by this one.
 - Add patch 8 to save the GIC IRQ in mtk_pcie_port as a prepare patch for tear
   down the irq when remove the kernel module.
 - Re-arrange the find_port flow suggest by Lorenzo to make the code parse easier
   for the patch 1.
 - Remove the .pm_support in mtk_pcie_soc in patch 7 since if system pm was not
   supported, then those pm callbacks will never be executed for MT7622. So the
   .pm_support is not needed.

Change since v3:
 - Remove pm_runtime_XXX ops in suspend and resume callbacks in the third patch.
 - Rebase to 4.19-rc1.

Change since v2:
 - Fix the list_for_each_entry_safe parameter error.
 - Add Ryder's Acked-by flag.

Change since v1:
 - A bit of code refact of the first patch suggested by Andy Shevchenko, and
   commit message updated.
 - Using __maybe_unused.
 - Remove the redundant list_empty check of the fourth patch.

Honghui Zhang (9):
  PCI: mediatek: Using slot's devfn for compare to fix
    mtk_pcie_find_port logic
  PCI: mediatek: Fixup class ID for MT7622 as PCI_CLASS_BRIDGE_PCI
  PCI: mediatek: Remove the redundant dev->pm_domain check
  PCI: mediatek: Convert to use pci_host_probe()
  PCI: mediatek: Move the mtk_pcie_startup_port_v2 function's define
    after mtk_pcie_setup_irq
  PCI: mediatek: Enable msi after clock enabled
  PCI: mediatek: Add system pm support for MT2712 and MT7622
  PCI: mediatek: Save the GIC IRQ in mtk_pcie_port
  PCI: mediatek: Add loadable kernel module support

 drivers/pci/controller/Kconfig         |   2 +-
 drivers/pci/controller/pcie-mediatek.c | 326 +++++++++++++++++++++------------
 2 files changed, 210 insertions(+), 118 deletions(-)

-- 
2.6.4

[PATCH v5 1/9] PCI: mediatek: Using slot's devfn for compare to fix mtk_pcie_find_port logic

[honghui.zhang]

From: Honghui Zhang <honghui.zhang@mediatek.com>

The Mediatek's host controller has two slots, each with it's own control
registers. The host driver need to identify which slot was connected
in order to access the device's configuration space. There's problem
for current host driver to find out which slot was connected to for
a given EP device.

Assuming each slot have connect with one EP device as below:

                host bridge
  bus 0 --> __________|_______
           |                  |
           |                  |
         slot 0             slot 1
  bus 1 -->|        bus 2 --> |
           |                  |
         EP 0               EP 1

During PCI enumeration, system software will scan all the PCI device
starting from devfn 0. So it will get the proper port for slot0 and
slot1 device when using PCI_SLOT(devfn) for match. But it will get
the wrong slot for EP1: The devfn will be start from 0 when scanning
EP1 behind slot1, it will get port0 since the PCI_SLOT(EP1) is match
for port0's slot value. So the host driver should not using EP's devfn
but the slot's devfn(the slot which EP was connected to) for match.

This patch fix the mtk_pcie_find_port's logic by using the slot's
devfn for match if finding device connected to the subordinate bus.

Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
---
 drivers/pci/controller/pcie-mediatek.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index 9999dae..264e03f 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -337,10 +337,25 @@ static struct mtk_pcie_port *mtk_pcie_find_port(struct pci_bus *bus,
 {
 	struct mtk_pcie *pcie = bus->sysdata;
 	struct mtk_pcie_port *port;
+	struct pci_dev *dev = NULL;
+
+	/*
+	 * Walk the bus hierarchy to get the devfn value
+	 * of the port in the root bus.
+	 */
+	while (bus && bus->number) {
+		dev = bus->self;
+		bus = dev->bus;
+	}
+
+	list_for_each_entry(port, &pcie->ports, list) {
+		/* Using slot's devfn to compare for subordinary bus. */
+		if (dev)
+			devfn = dev->devfn;
 
-	list_for_each_entry(port, &pcie->ports, list)
 		if (port->slot == PCI_SLOT(devfn))
 			return port;
+	}
 
 	return NULL;
 }
-- 
2.6.4

[PATCH v5 2/9] PCI: mediatek: Fixup class ID for MT7622 as PCI_CLASS_BRIDGE_PCI

[honghui.zhang]

From: Honghui Zhang <honghui.zhang@mediatek.com>

The PCIe controller of MT7622 has TYPE 1 configuration space type, but
the HW default class type values is invalid.

The commit 101c92dc80c8 ("PCI: mediatek: Set up vendor ID and class
type for MT7622") have set the class ID for MT7622 as
PCI_CLASS_BRIDGE_HOST, but it's not workable for MT7622:

In __pci_bus_assign_resources, the framework only setup bridge's
resource window only if class type is PCI_CLASS_BRIDGE_PCI. Or it
will leave the subordinary PCIe device's MMIO window un-touched.

Fixup the class type to PCI_CLASS_BRIDGE_PCI as most of the controller
driver do.

Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
---
 drivers/pci/controller/pcie-mediatek.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index 264e03f..3ab80d6 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -436,7 +436,7 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
 		val = PCI_VENDOR_ID_MEDIATEK;
 		writew(val, port->base + PCIE_CONF_VEND_ID);
 
-		val = PCI_CLASS_BRIDGE_HOST;
+		val = PCI_CLASS_BRIDGE_PCI;
 		writew(val, port->base + PCIE_CONF_CLASS_ID);
 	}
 
-- 
2.6.4

[PATCH v5 3/9] PCI: mediatek: Remove the redundant dev->pm_domain check

[honghui.zhang]

From: Honghui Zhang <honghui.zhang@mediatek.com>

It's no needed to check whether device have pm_domain attached before
calling the pm_runtime_XXX interface, remove it.

Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
---
 drivers/pci/controller/pcie-mediatek.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index 3ab80d6..e2c4127 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -225,10 +225,8 @@ static void mtk_pcie_subsys_powerdown(struct mtk_pcie *pcie)
 
 	clk_disable_unprepare(pcie->free_ck);
 
-	if (dev->pm_domain) {
-		pm_runtime_put_sync(dev);
-		pm_runtime_disable(dev);
-	}
+	pm_runtime_put_sync(dev);
+	pm_runtime_disable(dev);
 }
 
 static void mtk_pcie_port_free(struct mtk_pcie_port *port)
@@ -1002,10 +1000,8 @@ static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie)
 		pcie->free_ck = NULL;
 	}
 
-	if (dev->pm_domain) {
-		pm_runtime_enable(dev);
-		pm_runtime_get_sync(dev);
-	}
+	pm_runtime_enable(dev);
+	pm_runtime_get_sync(dev);
 
 	/* enable top level clock */
 	err = clk_prepare_enable(pcie->free_ck);
@@ -1017,10 +1013,8 @@ static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie)
 	return 0;
 
 err_free_ck:
-	if (dev->pm_domain) {
-		pm_runtime_put_sync(dev);
-		pm_runtime_disable(dev);
-	}
+	pm_runtime_put_sync(dev);
+	pm_runtime_disable(dev);
 
 	return err;
 }
-- 
2.6.4

[PATCH v5 4/9] PCI: mediatek: Convert to use pci_host_probe()

[honghui.zhang]

From: Honghui Zhang <honghui.zhang@mediatek.com>

Part of mtk_pcie_register_host is an open-coded version of
pci_host_probe(). So instead of duplicating this code, use
pci_host_probe() directly and remove mtk_pcie_register_host.

Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
---
 drivers/pci/controller/pcie-mediatek.c | 37 ++++++++--------------------------
 1 file changed, 8 insertions(+), 29 deletions(-)

diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index e2c4127..cbf4543 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -1125,34 +1125,6 @@ static int mtk_pcie_request_resources(struct mtk_pcie *pcie)
 	return 0;
 }
 
-static int mtk_pcie_register_host(struct pci_host_bridge *host)
-{
-	struct mtk_pcie *pcie = pci_host_bridge_priv(host);
-	struct pci_bus *child;
-	int err;
-
-	host->busnr = pcie->busn.start;
-	host->dev.parent = pcie->dev;
-	host->ops = pcie->soc->ops;
-	host->map_irq = of_irq_parse_and_map_pci;
-	host->swizzle_irq = pci_common_swizzle;
-	host->sysdata = pcie;
-
-	err = pci_scan_root_bus_bridge(host);
-	if (err < 0)
-		return err;
-
-	pci_bus_size_bridges(host->bus);
-	pci_bus_assign_resources(host->bus);
-
-	list_for_each_entry(child, &host->bus->children, node)
-		pcie_bus_configure_settings(child);
-
-	pci_bus_add_devices(host->bus);
-
-	return 0;
-}
-
 static int mtk_pcie_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -1179,7 +1151,14 @@ static int mtk_pcie_probe(struct platform_device *pdev)
 	if (err)
 		goto put_resources;
 
-	err = mtk_pcie_register_host(host);
+	host->busnr = pcie->busn.start;
+	host->dev.parent = pcie->dev;
+	host->ops = pcie->soc->ops;
+	host->map_irq = of_irq_parse_and_map_pci;
+	host->swizzle_irq = pci_common_swizzle;
+	host->sysdata = pcie;
+
+	err = pci_host_probe(host);
 	if (err)
 		goto put_resources;
 
-- 
2.6.4

[PATCH v5 5/9] PCI: mediatek: Move the mtk_pcie_startup_port_v2 function's define after mtk_pcie_setup_irq

[honghui.zhang]

From: Honghui Zhang <honghui.zhang@mediatek.com>

This is a prepare patch to fix enable MSI logic, move the function's
define later to avoid forward declaration of mtk_pcie_enable_msi in
the future. No functional changed.

Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
---
 drivers/pci/controller/pcie-mediatek.c | 138 ++++++++++++++++-----------------
 1 file changed, 69 insertions(+), 69 deletions(-)

diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index cbf4543..d150be1 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -396,75 +396,6 @@ static struct pci_ops mtk_pcie_ops_v2 = {
 	.write = mtk_pcie_config_write,
 };
 
-static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
-{
-	struct mtk_pcie *pcie = port->pcie;
-	struct resource *mem = &pcie->mem;
-	const struct mtk_pcie_soc *soc = port->pcie->soc;
-	u32 val;
-	size_t size;
-	int err;
-
-	/* MT7622 platforms need to enable LTSSM and ASPM from PCIe subsys */
-	if (pcie->base) {
-		val = readl(pcie->base + PCIE_SYS_CFG_V2);
-		val |= PCIE_CSR_LTSSM_EN(port->slot) |
-		       PCIE_CSR_ASPM_L1_EN(port->slot);
-		writel(val, pcie->base + PCIE_SYS_CFG_V2);
-	}
-
-	/* Assert all reset signals */
-	writel(0, port->base + PCIE_RST_CTRL);
-
-	/*
-	 * Enable PCIe link down reset, if link status changed from link up to
-	 * link down, this will reset MAC control registers and configuration
-	 * space.
-	 */
-	writel(PCIE_LINKDOWN_RST_EN, port->base + PCIE_RST_CTRL);
-
-	/* De-assert PHY, PE, PIPE, MAC and configuration reset	*/
-	val = readl(port->base + PCIE_RST_CTRL);
-	val |= PCIE_PHY_RSTB | PCIE_PERSTB | PCIE_PIPE_SRSTB |
-	       PCIE_MAC_SRSTB | PCIE_CRSTB;
-	writel(val, port->base + PCIE_RST_CTRL);
-
-	/* Set up vendor ID and class code */
-	if (soc->need_fix_class_id) {
-		val = PCI_VENDOR_ID_MEDIATEK;
-		writew(val, port->base + PCIE_CONF_VEND_ID);
-
-		val = PCI_CLASS_BRIDGE_PCI;
-		writew(val, port->base + PCIE_CONF_CLASS_ID);
-	}
-
-	/* 100ms timeout value should be enough for Gen1/2 training */
-	err = readl_poll_timeout(port->base + PCIE_LINK_STATUS_V2, val,
-				 !!(val & PCIE_PORT_LINKUP_V2), 20,
-				 100 * USEC_PER_MSEC);
-	if (err)
-		return -ETIMEDOUT;
-
-	/* Set INTx mask */
-	val = readl(port->base + PCIE_INT_MASK);
-	val &= ~INTX_MASK;
-	writel(val, port->base + PCIE_INT_MASK);
-
-	/* Set AHB to PCIe translation windows */
-	size = mem->end - mem->start;
-	val = lower_32_bits(mem->start) | AHB2PCIE_SIZE(fls(size));
-	writel(val, port->base + PCIE_AHB_TRANS_BASE0_L);
-
-	val = upper_32_bits(mem->start);
-	writel(val, port->base + PCIE_AHB_TRANS_BASE0_H);
-
-	/* Set PCIe to AXI translation memory space.*/
-	val = fls(0xffffffff) | WIN_ENABLE;
-	writel(val, port->base + PCIE_AXI_WINDOW0);
-
-	return 0;
-}
-
 static void mtk_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
 {
 	struct mtk_pcie_port *port = irq_data_get_irq_chip_data(data);
@@ -709,6 +640,75 @@ static int mtk_pcie_setup_irq(struct mtk_pcie_port *port,
 	return 0;
 }
 
+static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
+{
+	struct mtk_pcie *pcie = port->pcie;
+	struct resource *mem = &pcie->mem;
+	const struct mtk_pcie_soc *soc = port->pcie->soc;
+	u32 val;
+	size_t size;
+	int err;
+
+	/* MT7622 platforms need to enable LTSSM and ASPM from PCIe subsys */
+	if (pcie->base) {
+		val = readl(pcie->base + PCIE_SYS_CFG_V2);
+		val |= PCIE_CSR_LTSSM_EN(port->slot) |
+		       PCIE_CSR_ASPM_L1_EN(port->slot);
+		writel(val, pcie->base + PCIE_SYS_CFG_V2);
+	}
+
+	/* Assert all reset signals */
+	writel(0, port->base + PCIE_RST_CTRL);
+
+	/*
+	 * Enable PCIe link down reset, if link status changed from link up to
+	 * link down, this will reset MAC control registers and configuration
+	 * space.
+	 */
+	writel(PCIE_LINKDOWN_RST_EN, port->base + PCIE_RST_CTRL);
+
+	/* De-assert PHY, PE, PIPE, MAC and configuration reset	*/
+	val = readl(port->base + PCIE_RST_CTRL);
+	val |= PCIE_PHY_RSTB | PCIE_PERSTB | PCIE_PIPE_SRSTB |
+	       PCIE_MAC_SRSTB | PCIE_CRSTB;
+	writel(val, port->base + PCIE_RST_CTRL);
+
+	/* Set up vendor ID and class code */
+	if (soc->need_fix_class_id) {
+		val = PCI_VENDOR_ID_MEDIATEK;
+		writew(val, port->base + PCIE_CONF_VEND_ID);
+
+		val = PCI_CLASS_BRIDGE_PCI;
+		writew(val, port->base + PCIE_CONF_CLASS_ID);
+	}
+
+	/* 100ms timeout value should be enough for Gen1/2 training */
+	err = readl_poll_timeout(port->base + PCIE_LINK_STATUS_V2, val,
+				 !!(val & PCIE_PORT_LINKUP_V2), 20,
+				 100 * USEC_PER_MSEC);
+	if (err)
+		return -ETIMEDOUT;
+
+	/* Set INTx mask */
+	val = readl(port->base + PCIE_INT_MASK);
+	val &= ~INTX_MASK;
+	writel(val, port->base + PCIE_INT_MASK);
+
+	/* Set AHB to PCIe translation windows */
+	size = mem->end - mem->start;
+	val = lower_32_bits(mem->start) | AHB2PCIE_SIZE(fls(size));
+	writel(val, port->base + PCIE_AHB_TRANS_BASE0_L);
+
+	val = upper_32_bits(mem->start);
+	writel(val, port->base + PCIE_AHB_TRANS_BASE0_H);
+
+	/* Set PCIe to AXI translation memory space.*/
+	val = fls(0xffffffff) | WIN_ENABLE;
+	writel(val, port->base + PCIE_AXI_WINDOW0);
+
+	return 0;
+}
+
 static void __iomem *mtk_pcie_map_bus(struct pci_bus *bus,
 				      unsigned int devfn, int where)
 {
-- 
2.6.4

[PATCH v5 6/9] PCI: mediatek: Enable msi after clock enabled

[honghui.zhang]

From: Honghui Zhang <honghui.zhang@mediatek.com>

The clocks was not enabled when enable MSI. This patch fix this
issue by calling mtk_pcie_enable_msi in mtk_pcie_startup_port_v2
since the clock was all enabled at that time.

Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
---
 drivers/pci/controller/pcie-mediatek.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index d150be1..be38b38 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -572,8 +572,6 @@ static int mtk_pcie_init_irq_domain(struct mtk_pcie_port *port,
 		ret = mtk_pcie_allocate_msi_domains(port);
 		if (ret)
 			return ret;
-
-		mtk_pcie_enable_msi(port);
 	}
 
 	return 0;
@@ -694,6 +692,9 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
 	val &= ~INTX_MASK;
 	writel(val, port->base + PCIE_INT_MASK);
 
+	if (IS_ENABLED(CONFIG_PCI_MSI))
+		mtk_pcie_enable_msi(port);
+
 	/* Set AHB to PCIe translation windows */
 	size = mem->end - mem->start;
 	val = lower_32_bits(mem->start) | AHB2PCIE_SIZE(fls(size));
-- 
2.6.4

[PATCH v5 7/9] PCI: mediatek: Add system pm support for MT2712 and MT7622

[honghui.zhang]

From: Honghui Zhang <honghui.zhang@mediatek.com>

In order to reduce the PCIe power consuming while system suspend,
the physical layer should be gated. And the PCIe link should be
re-established and the related control register values should be
re-initialized after system resume.

Register suspend_noirq & resume_noirq callback functions to allow
PCIe to come up after resume from RAM.

Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
---
 drivers/pci/controller/pcie-mediatek.c | 50 ++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index be38b38..bd1bde3 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -1172,6 +1172,55 @@ static int mtk_pcie_probe(struct platform_device *pdev)
 	return err;
 }
 
+static int __maybe_unused mtk_pcie_suspend_noirq(struct device *dev)
+{
+	struct mtk_pcie *pcie = dev_get_drvdata(dev);
+	struct mtk_pcie_port *port;
+
+	if (list_empty(&pcie->ports))
+		return 0;
+
+	list_for_each_entry(port, &pcie->ports, list) {
+		clk_disable_unprepare(port->pipe_ck);
+		clk_disable_unprepare(port->obff_ck);
+		clk_disable_unprepare(port->axi_ck);
+		clk_disable_unprepare(port->aux_ck);
+		clk_disable_unprepare(port->ahb_ck);
+		clk_disable_unprepare(port->sys_ck);
+		phy_power_off(port->phy);
+		phy_exit(port->phy);
+	}
+
+	clk_disable_unprepare(pcie->free_ck);
+
+	return 0;
+}
+
+static int __maybe_unused mtk_pcie_resume_noirq(struct device *dev)
+{
+	struct mtk_pcie *pcie = dev_get_drvdata(dev);
+	struct mtk_pcie_port *port, *tmp;
+
+	if (list_empty(&pcie->ports))
+		return 0;
+
+	clk_prepare_enable(pcie->free_ck);
+
+	list_for_each_entry_safe(port, tmp, &pcie->ports, list)
+		mtk_pcie_enable_port(port);
+
+	/* In case of EP was removed while system suspend. */
+	if (list_empty(&pcie->ports))
+		clk_disable_unprepare(pcie->free_ck);
+
+	return 0;
+}
+
+static const struct dev_pm_ops mtk_pcie_pm_ops = {
+	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(mtk_pcie_suspend_noirq,
+				      mtk_pcie_resume_noirq)
+};
+
 static const struct mtk_pcie_soc mtk_pcie_soc_v1 = {
 	.ops = &mtk_pcie_ops,
 	.startup = mtk_pcie_startup_port,
@@ -1204,6 +1253,7 @@ static struct platform_driver mtk_pcie_driver = {
 		.name = "mtk-pcie",
 		.of_match_table = mtk_pcie_ids,
 		.suppress_bind_attrs = true,
+		.pm = &mtk_pcie_pm_ops,
 	},
 };
 builtin_platform_driver(mtk_pcie_driver);
-- 
2.6.4

[PATCH v5 8/9] PCI: mediatek: Save the GIC IRQ in mtk_pcie_port

[honghui.zhang]

From: Honghui Zhang <honghui.zhang@mediatek.com>

Need to save the PCIe's GIC IRQ for dispose_irq, this is a prepare
patch for add mediatek PCIe module support to tear down the IRQ, no
functional changed.

Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
---
 drivers/pci/controller/pcie-mediatek.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index bd1bde3..d35890c 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -162,6 +162,7 @@ struct mtk_pcie_soc {
  * @phy: pointer to PHY control block
  * @lane: lane count
  * @slot: port slot
+ * @irq: GIC irq
  * @irq_domain: legacy INTx IRQ domain
  * @inner_domain: inner IRQ domain
  * @msi_domain: MSI IRQ domain
@@ -182,6 +183,7 @@ struct mtk_pcie_port {
 	struct phy *phy;
 	u32 lane;
 	u32 slot;
+	int irq;
 	struct irq_domain *irq_domain;
 	struct irq_domain *inner_domain;
 	struct irq_domain *msi_domain;
@@ -624,7 +626,7 @@ static int mtk_pcie_setup_irq(struct mtk_pcie_port *port,
 	struct mtk_pcie *pcie = port->pcie;
 	struct device *dev = pcie->dev;
 	struct platform_device *pdev = to_platform_device(dev);
-	int err, irq;
+	int err;
 
 	err = mtk_pcie_init_irq_domain(port, node);
 	if (err) {
@@ -632,8 +634,9 @@ static int mtk_pcie_setup_irq(struct mtk_pcie_port *port,
 		return err;
 	}
 
-	irq = platform_get_irq(pdev, port->slot);
-	irq_set_chained_handler_and_data(irq, mtk_pcie_intr_handler, port);
+	port->irq = platform_get_irq(pdev, port->slot);
+	irq_set_chained_handler_and_data(port->irq,
+					 mtk_pcie_intr_handler, port);
 
 	return 0;
 }
-- 
2.6.4

[PATCH v5 9/9] PCI: mediatek: Add loadable kernel module support

[honghui.zhang]

From: Honghui Zhang <honghui.zhang@mediatek.com>

Implement remove callback function for Mediatek PCIe driver to add
loadable kernel module support.

Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
---
 drivers/pci/controller/Kconfig         |  2 +-
 drivers/pci/controller/pcie-mediatek.c | 52 +++++++++++++++++++++++++++++++++-
 2 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index 028b287..465790f 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -231,7 +231,7 @@ config PCIE_ROCKCHIP_EP
 	  available to support GEN2 with 4 slots.
 
 config PCIE_MEDIATEK
-	bool "MediaTek PCIe controller"
+	tristate "MediaTek PCIe controller"
 	depends on ARCH_MEDIATEK || COMPILE_TEST
 	depends on OF
 	depends on PCI_MSI_IRQ_DOMAIN
diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index d35890c..b8ae214 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -15,6 +15,7 @@
 #include <linux/irqdomain.h>
 #include <linux/kernel.h>
 #include <linux/msi.h>
+#include <linux/module.h>
 #include <linux/of_address.h>
 #include <linux/of_pci.h>
 #include <linux/of_platform.h>
@@ -536,6 +537,27 @@ static void mtk_pcie_enable_msi(struct mtk_pcie_port *port)
 	writel(val, port->base + PCIE_INT_MASK);
 }
 
+static void mtk_pcie_irq_teardown(struct mtk_pcie *pcie)
+{
+	struct mtk_pcie_port *port, *tmp;
+
+	list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
+		irq_set_chained_handler_and_data(port->irq, NULL, NULL);
+
+		if (port->irq_domain)
+			irq_domain_remove(port->irq_domain);
+
+		if (IS_ENABLED(CONFIG_PCI_MSI)) {
+			if (port->msi_domain)
+				irq_domain_remove(port->msi_domain);
+			if (port->inner_domain)
+				irq_domain_remove(port->inner_domain);
+		}
+
+		irq_dispose_mapping(port->irq);
+	}
+}
+
 static int mtk_pcie_intx_map(struct irq_domain *domain, unsigned int irq,
 			     irq_hw_number_t hwirq)
 {
@@ -1175,6 +1197,32 @@ static int mtk_pcie_probe(struct platform_device *pdev)
 	return err;
 }
 
+
+static void mtk_pcie_free_resources(struct mtk_pcie *pcie)
+{
+	struct pci_host_bridge *host = pci_host_bridge_from_priv(pcie);
+	struct list_head *windows = &host->windows;
+
+	pci_unmap_iospace(&pcie->pio);
+	pci_free_resource_list(windows);
+}
+
+static int mtk_pcie_remove(struct platform_device *pdev)
+{
+	struct mtk_pcie *pcie = platform_get_drvdata(pdev);
+	struct pci_host_bridge *host = pci_host_bridge_from_priv(pcie);
+
+	pci_stop_root_bus(host->bus);
+	pci_remove_root_bus(host->bus);
+	mtk_pcie_free_resources(pcie);
+
+	mtk_pcie_irq_teardown(pcie);
+
+	mtk_pcie_put_resources(pcie);
+
+	return 0;
+}
+
 static int __maybe_unused mtk_pcie_suspend_noirq(struct device *dev)
 {
 	struct mtk_pcie *pcie = dev_get_drvdata(dev);
@@ -1252,6 +1300,7 @@ static const struct of_device_id mtk_pcie_ids[] = {
 
 static struct platform_driver mtk_pcie_driver = {
 	.probe = mtk_pcie_probe,
+	.remove = mtk_pcie_remove,
 	.driver = {
 		.name = "mtk-pcie",
 		.of_match_table = mtk_pcie_ids,
@@ -1259,4 +1308,5 @@ static struct platform_driver mtk_pcie_driver = {
 		.pm = &mtk_pcie_pm_ops,
 	},
 };
-builtin_platform_driver(mtk_pcie_driver);
+module_platform_driver(mtk_pcie_driver);
+MODULE_LICENSE("GPL v2");
-- 
2.6.4

Re: [PATCH v5 4/9] PCI: mediatek: Convert to use pci_host_probe()

[Ryder Lee]

On Fri, 2018-09-28 at 18:04 +0800, honghui.zhang@mediatek.com wrote:
> From: Honghui Zhang <honghui.zhang@mediatek.com>
> 
> Part of mtk_pcie_register_host is an open-coded version of
> pci_host_probe(). So instead of duplicating this code, use
> pci_host_probe() directly and remove mtk_pcie_register_host.
> 
> Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
> ---
>  drivers/pci/controller/pcie-mediatek.c | 37 ++++++++--------------------------
>  1 file changed, 8 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index e2c4127..cbf4543 100644
> --- a/drivers/pci/controller/pcie-mediatek.c
> +++ b/drivers/pci/controller/pcie-mediatek.c
> @@ -1125,34 +1125,6 @@ static int mtk_pcie_request_resources(struct mtk_pcie *pcie)
>  	return 0;
>  }
>  
> -static int mtk_pcie_register_host(struct pci_host_bridge *host)
> -{
> -	struct mtk_pcie *pcie = pci_host_bridge_priv(host);
> -	struct pci_bus *child;
> -	int err;

To make the patch simple, could we keep these host->*. and return
pci_host_probe() directly?

> -	host->busnr = pcie->busn.start;
> -	host->dev.parent = pcie->dev;
> -	host->ops = pcie->soc->ops;
> -	host->map_irq = of_irq_parse_and_map_pci;
> -	host->swizzle_irq = pci_common_swizzle;
> -	host->sysdata = pcie;
> -
> -	err = pci_scan_root_bus_bridge(host);
> -	if (err < 0)
> -		return err;
> -
> -	pci_bus_size_bridges(host->bus);
> -	pci_bus_assign_resources(host->bus);
> -
> -	list_for_each_entry(child, &host->bus->children, node)
> -		pcie_bus_configure_settings(child);
> -
> -	pci_bus_add_devices(host->bus);
> -
> -	return 0;
> -}
> -
>  static int mtk_pcie_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -1179,7 +1151,14 @@ static int mtk_pcie_probe(struct platform_device *pdev)
>  	if (err)
>  		goto put_resources;
>  
> -	err = mtk_pcie_register_host(host);
> +	host->busnr = pcie->busn.start;
> +	host->dev.parent = pcie->dev;
> +	host->ops = pcie->soc->ops;
> +	host->map_irq = of_irq_parse_and_map_pci;
> +	host->swizzle_irq = pci_common_swizzle;
> +	host->sysdata = pcie;
> +
> +	err = pci_host_probe(host);
>  	if (err)
>  		goto put_resources;
>  

Re: [PATCH v5 4/9] PCI: mediatek: Convert to use pci_host_probe()

[Honghui Zhang]

On Fri, 2018-09-28 at 23:41 +0800, Ryder Lee wrote:
> On Fri, 2018-09-28 at 18:04 +0800, honghui.zhang@mediatek.com wrote:
> > From: Honghui Zhang <honghui.zhang@mediatek.com>
> > 
> > Part of mtk_pcie_register_host is an open-coded version of
> > pci_host_probe(). So instead of duplicating this code, use
> > pci_host_probe() directly and remove mtk_pcie_register_host.
> > 
> > Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
> > ---
> >  drivers/pci/controller/pcie-mediatek.c | 37 ++++++++--------------------------
> >  1 file changed, 8 insertions(+), 29 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> > index e2c4127..cbf4543 100644
> > --- a/drivers/pci/controller/pcie-mediatek.c
> > +++ b/drivers/pci/controller/pcie-mediatek.c
> > @@ -1125,34 +1125,6 @@ static int mtk_pcie_request_resources(struct mtk_pcie *pcie)
> >  	return 0;
> >  }
> >  
> > -static int mtk_pcie_register_host(struct pci_host_bridge *host)
> > -{
> > -	struct mtk_pcie *pcie = pci_host_bridge_priv(host);
> > -	struct pci_bus *child;
> > -	int err;
> 
> To make the patch simple, could we keep these host->*. and return
> pci_host_probe() directly?

Well, I guess it's not necessary to keep the mtk_pcie_register_host just
wrap pci_host_probe(). Put those host->* directly in probe function is
no harm, it will reduce one function call and related parameter passing.

I may update this patch to keep the mtk_pcie_register_host if you insist
though I prefer the current way.

Thanks.
> 
> > -	host->busnr = pcie->busn.start;
> > -	host->dev.parent = pcie->dev;
> > -	host->ops = pcie->soc->ops;
> > -	host->map_irq = of_irq_parse_and_map_pci;
> > -	host->swizzle_irq = pci_common_swizzle;
> > -	host->sysdata = pcie;
> > -
> > -	err = pci_scan_root_bus_bridge(host);
> > -	if (err < 0)
> > -		return err;
> > -
> > -	pci_bus_size_bridges(host->bus);
> > -	pci_bus_assign_resources(host->bus);
> > -
> > -	list_for_each_entry(child, &host->bus->children, node)
> > -		pcie_bus_configure_settings(child);
> > -
> > -	pci_bus_add_devices(host->bus);
> > -
> > -	return 0;
> > -}
> > -
> >  static int mtk_pcie_probe(struct platform_device *pdev)
> >  {
> >  	struct device *dev = &pdev->dev;
> > @@ -1179,7 +1151,14 @@ static int mtk_pcie_probe(struct platform_device *pdev)
> >  	if (err)
> >  		goto put_resources;
> >  
> > -	err = mtk_pcie_register_host(host);
> > +	host->busnr = pcie->busn.start;
> > +	host->dev.parent = pcie->dev;
> > +	host->ops = pcie->soc->ops;
> > +	host->map_irq = of_irq_parse_and_map_pci;
> > +	host->swizzle_irq = pci_common_swizzle;
> > +	host->sysdata = pcie;
> > +
> > +	err = pci_host_probe(host);
> >  	if (err)
> >  		goto put_resources;
> >  
> 
> 

Re: [PATCH v5 4/9] PCI: mediatek: Convert to use pci_host_probe()

[Ryder Lee]

On Sat, 2018-09-29 at 08:16 +0800, Honghui Zhang wrote:
> On Fri, 2018-09-28 at 23:41 +0800, Ryder Lee wrote:
> > On Fri, 2018-09-28 at 18:04 +0800, honghui.zhang@mediatek.com wrote:
> > > From: Honghui Zhang <honghui.zhang@mediatek.com>
> > > 
> > > Part of mtk_pcie_register_host is an open-coded version of
> > > pci_host_probe(). So instead of duplicating this code, use
> > > pci_host_probe() directly and remove mtk_pcie_register_host.
> > > 
> > > Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
> > > ---
> > >  drivers/pci/controller/pcie-mediatek.c | 37 ++++++++--------------------------
> > >  1 file changed, 8 insertions(+), 29 deletions(-)
> > > 
> > > diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> > > index e2c4127..cbf4543 100644
> > > --- a/drivers/pci/controller/pcie-mediatek.c
> > > +++ b/drivers/pci/controller/pcie-mediatek.c
> > > @@ -1125,34 +1125,6 @@ static int mtk_pcie_request_resources(struct mtk_pcie *pcie)
> > >  	return 0;
> > >  }
> > >  
> > > -static int mtk_pcie_register_host(struct pci_host_bridge *host)
> > > -{
> > > -	struct mtk_pcie *pcie = pci_host_bridge_priv(host);
> > > -	struct pci_bus *child;
> > > -	int err;
> > 
> > To make the patch simple, could we keep these host->*. and return
> > pci_host_probe() directly?
> 
> Well, I guess it's not necessary to keep the mtk_pcie_register_host just
> wrap pci_host_probe(). Put those host->* directly in probe function is
> no harm, it will reduce one function call and related parameter passing.
> 
> I may update this patch to keep the mtk_pcie_register_host if you insist
> though I prefer the current way.
> 
> Thanks.

Okay. I'm fine with this.

> > > -	host->busnr = pcie->busn.start;
> > > -	host->dev.parent = pcie->dev;
> > > -	host->ops = pcie->soc->ops;
> > > -	host->map_irq = of_irq_parse_and_map_pci;
> > > -	host->swizzle_irq = pci_common_swizzle;
> > > -	host->sysdata = pcie;
> > > -
> > > -	err = pci_scan_root_bus_bridge(host);
> > > -	if (err < 0)
> > > -		return err;
> > > -
> > > -	pci_bus_size_bridges(host->bus);
> > > -	pci_bus_assign_resources(host->bus);
> > > -
> > > -	list_for_each_entry(child, &host->bus->children, node)
> > > -		pcie_bus_configure_settings(child);
> > > -
> > > -	pci_bus_add_devices(host->bus);
> > > -
> > > -	return 0;
> > > -}
> > > -
> > >  static int mtk_pcie_probe(struct platform_device *pdev)
> > >  {
> > >  	struct device *dev = &pdev->dev;
> > > @@ -1179,7 +1151,14 @@ static int mtk_pcie_probe(struct platform_device *pdev)
> > >  	if (err)
> > >  		goto put_resources;
> > >  
> > > -	err = mtk_pcie_register_host(host);
> > > +	host->busnr = pcie->busn.start;
> > > +	host->dev.parent = pcie->dev;
> > > +	host->ops = pcie->soc->ops;
> > > +	host->map_irq = of_irq_parse_and_map_pci;
> > > +	host->swizzle_irq = pci_common_swizzle;
> > > +	host->sysdata = pcie;
> > > +
> > > +	err = pci_host_probe(host);
> > >  	if (err)
> > >  		goto put_resources;
> > >  
> > 
> > 
> 
> 

Re: [PATCH v5 0/9] PCI: mediatek: fixup find_port, enable_msi and add pm, module support

[Ryder Lee]

On Fri, 2018-09-28 at 18:04 +0800, honghui.zhang@mediatek.com wrote:
> From: Honghui Zhang <honghui.zhang@mediatek.com>
> 
> This patchset includes misc patchs:
> 
> The patch 1 fixup the mtk_pcie_find_port logic which will cause system
> could not touch the EP's configuration space that connected to PCIe slot 1.
> 
> The patch 2 fixup the class type for MT7622.
> The patch 6 fixup the enable msi logic, the operation to enable msi
> should be after system clock is enabled. Call mtk_pcie_enable_msi in
> mtk_pcie_startup_port_v2 since the clock was all enabled at that time.
> 
> The patch 7 was rebased and refactor of the v4 patch[1], changes are:
>  -Add PM support for MT7622.
>  -Using mtk_pcie_enable_port to re-establish the link when resumed.
>  -Rebased on this patchset.
> 
> The patch 9 add loadable kernel module support.
> 
> [1] https://patchwork.kernel.org/patch/10479079
> 
> Change since v4:
>  - Add patch 2 to fixup class type for MT7622.
>  - Add patch 3 to remove the redundant dev->pm_domain check
>  - Add patch 4 to covert to use pci_host_probe
>  - Add patch 5 to re-arrange the function define, this is a prepare patch for
>    fixup the enable_msi logic, no functional changed have been made by this one.
>  - Add patch 8 to save the GIC IRQ in mtk_pcie_port as a prepare patch for tear
>    down the irq when remove the kernel module.
>  - Re-arrange the find_port flow suggest by Lorenzo to make the code parse easier
>    for the patch 1.
>  - Remove the .pm_support in mtk_pcie_soc in patch 7 since if system pm was not
>    supported, then those pm callbacks will never be executed for MT7622. So the
>    .pm_support is not needed.
> 
> Change since v3:
>  - Remove pm_runtime_XXX ops in suspend and resume callbacks in the third patch.
>  - Rebase to 4.19-rc1.
> 
> Change since v2:
>  - Fix the list_for_each_entry_safe parameter error.
>  - Add Ryder's Acked-by flag.
> 
> Change since v1:
>  - A bit of code refact of the first patch suggested by Andy Shevchenko, and
>    commit message updated.
>  - Using __maybe_unused.
>  - Remove the redundant list_empty check of the fourth patch.
> 
> Honghui Zhang (9):
>   PCI: mediatek: Using slot's devfn for compare to fix
>     mtk_pcie_find_port logic
>   PCI: mediatek: Fixup class ID for MT7622 as PCI_CLASS_BRIDGE_PCI
>   PCI: mediatek: Remove the redundant dev->pm_domain check
>   PCI: mediatek: Convert to use pci_host_probe()
>   PCI: mediatek: Move the mtk_pcie_startup_port_v2 function's define
>     after mtk_pcie_setup_irq
>   PCI: mediatek: Enable msi after clock enabled
>   PCI: mediatek: Add system pm support for MT2712 and MT7622
>   PCI: mediatek: Save the GIC IRQ in mtk_pcie_port
>   PCI: mediatek: Add loadable kernel module support
> 
>  drivers/pci/controller/Kconfig         |   2 +-
>  drivers/pci/controller/pcie-mediatek.c | 326 +++++++++++++++++++++------------
>  2 files changed, 210 insertions(+), 118 deletions(-)
> 

Acked-by: Ryder Lee <ryder.lee@mediatek.com> for the whole series.

Re: [PATCH v5 1/9] PCI: mediatek: Using slot's devfn for compare to fix mtk_pcie_find_port logic

[Lorenzo Pieralisi]

On Fri, Sep 28, 2018 at 06:04:32PM +0800, honghui.zhang@mediatek.com wrote:
> From: Honghui Zhang <honghui.zhang@mediatek.com>
> 
> The Mediatek's host controller has two slots, each with it's own control
> registers. The host driver need to identify which slot was connected
> in order to access the device's configuration space. There's problem
> for current host driver to find out which slot was connected to for
> a given EP device.
> 
> Assuming each slot have connect with one EP device as below:
> 
>                 host bridge
>   bus 0 --> __________|_______
>            |                  |
>            |                  |
>          slot 0             slot 1
>   bus 1 -->|        bus 2 --> |
>            |                  |
>          EP 0               EP 1
> 
> During PCI enumeration, system software will scan all the PCI device
> starting from devfn 0. So it will get the proper port for slot0 and
> slot1 device when using PCI_SLOT(devfn) for match. But it will get
> the wrong slot for EP1: The devfn will be start from 0 when scanning
> EP1 behind slot1, it will get port0 since the PCI_SLOT(EP1) is match
> for port0's slot value. So the host driver should not using EP's devfn
> but the slot's devfn(the slot which EP was connected to) for match.
> 
> This patch fix the mtk_pcie_find_port's logic by using the slot's
> devfn for match if finding device connected to the subordinate bus.
> 
> Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
> ---
>  drivers/pci/controller/pcie-mediatek.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index 9999dae..264e03f 100644
> --- a/drivers/pci/controller/pcie-mediatek.c
> +++ b/drivers/pci/controller/pcie-mediatek.c
> @@ -337,10 +337,25 @@ static struct mtk_pcie_port *mtk_pcie_find_port(struct pci_bus *bus,
>  {
>  	struct mtk_pcie *pcie = bus->sysdata;
>  	struct mtk_pcie_port *port;
> +	struct pci_dev *dev = NULL;
> +
> +	/*
> +	 * Walk the bus hierarchy to get the devfn value
> +	 * of the port in the root bus.
> +	 */
> +	while (bus && bus->number) {
> +		dev = bus->self;
> +		bus = dev->bus;

If you add:

devfn = dev->devfn;

here

> +	}
> +
> +	list_for_each_entry(port, &pcie->ports, list) {
> +		/* Using slot's devfn to compare for subordinary bus. */
> +		if (dev)
> +			devfn = dev->devfn;

You can remove this ugly hunk altogether (and dev initialization
to NULL).

Lorenzo

> -	list_for_each_entry(port, &pcie->ports, list)
>  		if (port->slot == PCI_SLOT(devfn))
>  			return port;
> +	}
>  
>  	return NULL;
>  }
> -- 
> 2.6.4
> 

Re: [PATCH v5 1/9] PCI: mediatek: Using slot's devfn for compare to fix mtk_pcie_find_port logic

[Lorenzo Pieralisi]

On Mon, Oct 01, 2018 at 03:36:41PM +0100, Lorenzo Pieralisi wrote:
> On Fri, Sep 28, 2018 at 06:04:32PM +0800, honghui.zhang@mediatek.com wrote:
> > From: Honghui Zhang <honghui.zhang@mediatek.com>
> > 
> > The Mediatek's host controller has two slots, each with it's own control
> > registers. The host driver need to identify which slot was connected
> > in order to access the device's configuration space. There's problem
> > for current host driver to find out which slot was connected to for
> > a given EP device.
> > 
> > Assuming each slot have connect with one EP device as below:
> > 
> >                 host bridge
> >   bus 0 --> __________|_______
> >            |                  |
> >            |                  |
> >          slot 0             slot 1
> >   bus 1 -->|        bus 2 --> |
> >            |                  |
> >          EP 0               EP 1
> > 
> > During PCI enumeration, system software will scan all the PCI device
> > starting from devfn 0. So it will get the proper port for slot0 and
> > slot1 device when using PCI_SLOT(devfn) for match. But it will get
> > the wrong slot for EP1: The devfn will be start from 0 when scanning
> > EP1 behind slot1, it will get port0 since the PCI_SLOT(EP1) is match
> > for port0's slot value. So the host driver should not using EP's devfn
> > but the slot's devfn(the slot which EP was connected to) for match.
> > 
> > This patch fix the mtk_pcie_find_port's logic by using the slot's
> > devfn for match if finding device connected to the subordinate bus.
> > 
> > Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
> > ---
> >  drivers/pci/controller/pcie-mediatek.c | 17 ++++++++++++++++-
> >  1 file changed, 16 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> > index 9999dae..264e03f 100644
> > --- a/drivers/pci/controller/pcie-mediatek.c
> > +++ b/drivers/pci/controller/pcie-mediatek.c
> > @@ -337,10 +337,25 @@ static struct mtk_pcie_port *mtk_pcie_find_port(struct pci_bus *bus,
> >  {
> >  	struct mtk_pcie *pcie = bus->sysdata;
> >  	struct mtk_pcie_port *port;
> > +	struct pci_dev *dev = NULL;
> > +
> > +	/*
> > +	 * Walk the bus hierarchy to get the devfn value
> > +	 * of the port in the root bus.
> > +	 */
> > +	while (bus && bus->number) {
> > +		dev = bus->self;
> > +		bus = dev->bus;
> 
> If you add:
> 
> devfn = dev->devfn;
> 
> here
> 
> > +	}
> > +
> > +	list_for_each_entry(port, &pcie->ports, list) {
> > +		/* Using slot's devfn to compare for subordinary bus. */
> > +		if (dev)
> > +			devfn = dev->devfn;
> 
> You can remove this ugly hunk altogether (and dev initialization
> to NULL).

Hi Honghui,

if you can make this change I would merge this series, it has been
been in the mailing lists for a while, I can make that change too
just let me know please.

Thanks,
Lorenzo

Re: [PATCH v5 6/9] PCI: mediatek: Enable msi after clock enabled

[Lorenzo Pieralisi]

On Fri, Sep 28, 2018 at 06:04:37PM +0800, honghui.zhang@mediatek.com wrote:
> From: Honghui Zhang <honghui.zhang@mediatek.com>
> 
> The clocks was not enabled when enable MSI. This patch fix this
> issue by calling mtk_pcie_enable_msi in mtk_pcie_startup_port_v2
> since the clock was all enabled at that time.
> 
> Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
> ---
>  drivers/pci/controller/pcie-mediatek.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

If this is a fix, and I think it is, I would appreciate a Fixes: tag
so that we can actully pinpoint the faulty commit.

Thanks,
Lorenzo

> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index d150be1..be38b38 100644
> --- a/drivers/pci/controller/pcie-mediatek.c
> +++ b/drivers/pci/controller/pcie-mediatek.c
> @@ -572,8 +572,6 @@ static int mtk_pcie_init_irq_domain(struct mtk_pcie_port *port,
>  		ret = mtk_pcie_allocate_msi_domains(port);
>  		if (ret)
>  			return ret;
> -
> -		mtk_pcie_enable_msi(port);
>  	}
>  
>  	return 0;
> @@ -694,6 +692,9 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
>  	val &= ~INTX_MASK;
>  	writel(val, port->base + PCIE_INT_MASK);
>  
> +	if (IS_ENABLED(CONFIG_PCI_MSI))
> +		mtk_pcie_enable_msi(port);
> +
>  	/* Set AHB to PCIe translation windows */
>  	size = mem->end - mem->start;
>  	val = lower_32_bits(mem->start) | AHB2PCIE_SIZE(fls(size));
> -- 
> 2.6.4
> 

Re: [PATCH v5 1/9] PCI: mediatek: Using slot's devfn for compare to fix mtk_pcie_find_port logic

[Honghui Zhang]

On Tue, 2018-10-02 at 11:59 +0100, Lorenzo Pieralisi wrote:
> On Mon, Oct 01, 2018 at 03:36:41PM +0100, Lorenzo Pieralisi wrote:
> > On Fri, Sep 28, 2018 at 06:04:32PM +0800, honghui.zhang@mediatek.com wrote:
> > > From: Honghui Zhang <honghui.zhang@mediatek.com>
> > > 
> > > The Mediatek's host controller has two slots, each with it's own control
> > > registers. The host driver need to identify which slot was connected
> > > in order to access the device's configuration space. There's problem
> > > for current host driver to find out which slot was connected to for
> > > a given EP device.
> > > 
> > > Assuming each slot have connect with one EP device as below:
> > > 
> > >                 host bridge
> > >   bus 0 --> __________|_______
> > >            |                  |
> > >            |                  |
> > >          slot 0             slot 1
> > >   bus 1 -->|        bus 2 --> |
> > >            |                  |
> > >          EP 0               EP 1
> > > 
> > > During PCI enumeration, system software will scan all the PCI device
> > > starting from devfn 0. So it will get the proper port for slot0 and
> > > slot1 device when using PCI_SLOT(devfn) for match. But it will get
> > > the wrong slot for EP1: The devfn will be start from 0 when scanning
> > > EP1 behind slot1, it will get port0 since the PCI_SLOT(EP1) is match
> > > for port0's slot value. So the host driver should not using EP's devfn
> > > but the slot's devfn(the slot which EP was connected to) for match.
> > > 
> > > This patch fix the mtk_pcie_find_port's logic by using the slot's
> > > devfn for match if finding device connected to the subordinate bus.
> > > 
> > > Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
> > > ---
> > >  drivers/pci/controller/pcie-mediatek.c | 17 ++++++++++++++++-
> > >  1 file changed, 16 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> > > index 9999dae..264e03f 100644
> > > --- a/drivers/pci/controller/pcie-mediatek.c
> > > +++ b/drivers/pci/controller/pcie-mediatek.c
> > > @@ -337,10 +337,25 @@ static struct mtk_pcie_port *mtk_pcie_find_port(struct pci_bus *bus,
> > >  {
> > >  	struct mtk_pcie *pcie = bus->sysdata;
> > >  	struct mtk_pcie_port *port;
> > > +	struct pci_dev *dev = NULL;
> > > +
> > > +	/*
> > > +	 * Walk the bus hierarchy to get the devfn value
> > > +	 * of the port in the root bus.
> > > +	 */
> > > +	while (bus && bus->number) {
> > > +		dev = bus->self;
> > > +		bus = dev->bus;
> > 
> > If you add:
> > 
> > devfn = dev->devfn;
> > 
> > here
> > 
> > > +	}
> > > +
> > > +	list_for_each_entry(port, &pcie->ports, list) {
> > > +		/* Using slot's devfn to compare for subordinary bus. */
> > > +		if (dev)
> > > +			devfn = dev->devfn;
> > 
> > You can remove this ugly hunk altogether (and dev initialization
> > to NULL).
> 
> Hi Honghui,
> 
> if you can make this change I would merge this series, it has been
> been in the mailing lists for a while, I can make that change too
> just let me know please.
> 

Hi, Lorenzo, sorry for the late reply.

I tried your advise and it works fine.

I will send another version to fix this, as well as to add a fix tag in
the commit message of patch 6.

thanks

> Thanks,
> Lorenzo