From: Honghui Zhang <[email protected]>
This patchset includes misc patchs:
The first patch fixup the mtk_pcie_find_port logical which will cause system
could not touch the EP's configuration space which was connected to PCIe slot 1.
The second patch fixup the enable msi logical, the operation to enable msi
should be after system clock is enabled. The function of mtk_pcie_startup_port_v2's
define location is re-arranged to avoid mtk_pcie_enable_msi's forward declaration.
And call mtk_pcie_enable_msi in mtk_pcie_startup_port_v2 since the clock was all
enabled at that time.
The third patch 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.
-Rebase on the previous two patches.
The fourth patch add loadable kernel module support.
Some of those patches was already reviewed-by Ryder Lee <[email protected]>,
so I just add the Reviewed-by tags in those patches.
[1] https://patchwork.kernel.org/patch/10479079
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 (4):
PCI: mediatek: fixup mtk_pcie_find_port logical
PCI: mediatek: enable msi after clock enabled
PCI: mediatek: Add system pm support for MT2712 and MT7622
PCI: mediatek: Add loadable kernel module support
drivers/pci/controller/Kconfig | 2 +-
drivers/pci/controller/pcie-mediatek.c | 284 ++++++++++++++++++++++++---------
2 files changed, 208 insertions(+), 78 deletions(-)
--
2.6.4
From: Honghui Zhang <[email protected]>
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 logical by using the slot's
devfn for match.
Signed-off-by: Honghui Zhang <[email protected]>
Acked-by: Ryder Lee <[email protected]>
---
drivers/pci/controller/pcie-mediatek.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index 861dda6..20b9088 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -337,11 +337,26 @@ 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;
+ struct pci_bus *pbus;
- list_for_each_entry(port, &pcie->ports, list)
- if (port->slot == PCI_SLOT(devfn))
+ list_for_each_entry(port, &pcie->ports, list) {
+ if (!bus->number && port->slot == PCI_SLOT(devfn))
return port;
+ if (bus->number) {
+ pbus = bus;
+
+ while (pbus->number) {
+ dev = pbus->self;
+ pbus = dev->bus;
+ }
+
+ if (port->slot == PCI_SLOT(dev->devfn))
+ return port;
+ }
+ }
+
return NULL;
}
--
2.6.4
From: Honghui Zhang <[email protected]>
Implement remove callback function for Mediatek PCIe driver to add
loadable kernel module support.
Signed-off-by: Honghui Zhang <[email protected]>
Reviewed-by: Ryder Lee <[email protected]>
---
drivers/pci/controller/Kconfig | 2 +-
drivers/pci/controller/pcie-mediatek.c | 60 +++++++++++++++++++++++++++++++---
2 files changed, 57 insertions(+), 5 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 bcc77bc..9d54fc9 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>
@@ -184,6 +185,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;
@@ -538,6 +540,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)
{
@@ -628,7 +651,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) {
@@ -636,8 +659,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;
}
@@ -1199,6 +1223,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);
@@ -1286,6 +1336,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,
@@ -1293,4 +1344,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
From: Honghui Zhang <[email protected]>
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.
The function of mtk_pcie_startup_port_v2's define location is
re-arranged to avoid mtk_pcie_enable_msi's forward declaration.
Signed-off-by: Honghui Zhang <[email protected]>
Reviewed-by: Ryder Lee <[email protected]>
---
drivers/pci/controller/pcie-mediatek.c | 143 +++++++++++++++++----------------
1 file changed, 72 insertions(+), 71 deletions(-)
diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index 20b9088..5aba43a 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -398,75 +398,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_HOST;
- 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);
@@ -643,8 +574,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;
@@ -711,6 +640,78 @@ 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_HOST;
+ 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);
+
+ 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));
+ 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
From: Honghui Zhang <[email protected]>
The MTCMOS of PCIe Host for MT2712 and MT7622 will be off when system
suspend, and all the internal control register will be reset after system
resume. The PCIe link should be re-established and the related control
register values should be re-set after system resume.
Signed-off-by: Honghui Zhang <[email protected]>
---
drivers/pci/controller/pcie-mediatek.c | 62 ++++++++++++++++++++++++++++++++++
1 file changed, 62 insertions(+)
diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index 5aba43a..bcc77bc 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -134,12 +134,14 @@ struct mtk_pcie_port;
/**
* struct mtk_pcie_soc - differentiate between host generations
* @need_fix_class_id: whether this host's class ID needed to be fixed or not
+ * @pm_support: whether the host's MTCMOS will be off when suspend
* @ops: pointer to configuration access functions
* @startup: pointer to controller setting functions
* @setup_irq: pointer to initialize IRQ functions
*/
struct mtk_pcie_soc {
bool need_fix_class_id;
+ bool pm_support;
struct pci_ops *ops;
int (*startup)(struct mtk_pcie_port *port);
int (*setup_irq)(struct mtk_pcie_port *port, struct device_node *node);
@@ -1197,12 +1199,70 @@ 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);
+ const struct mtk_pcie_soc *soc = pcie->soc;
+ struct mtk_pcie_port *port;
+
+ if (!soc->pm_support)
+ return 0;
+
+ 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);
+ const struct mtk_pcie_soc *soc = pcie->soc;
+ struct mtk_pcie_port *port, *tmp;
+
+ if (!soc->pm_support)
+ return 0;
+
+ 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,
};
static const struct mtk_pcie_soc mtk_pcie_soc_mt2712 = {
+ .pm_support = true,
.ops = &mtk_pcie_ops_v2,
.startup = mtk_pcie_startup_port_v2,
.setup_irq = mtk_pcie_setup_irq,
@@ -1210,6 +1270,7 @@ static const struct mtk_pcie_soc mtk_pcie_soc_mt2712 = {
static const struct mtk_pcie_soc mtk_pcie_soc_mt7622 = {
.need_fix_class_id = true,
+ .pm_support = true,
.ops = &mtk_pcie_ops_v2,
.startup = mtk_pcie_startup_port_v2,
.setup_irq = mtk_pcie_setup_irq,
@@ -1229,6 +1290,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
On Mon, Sep 10, 2018 at 05:50:20PM +0800, [email protected] wrote:
> From: Honghui Zhang <[email protected]>
>
> 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 logical by using the slot's
> devfn for match.
>
> Signed-off-by: Honghui Zhang <[email protected]>
> Acked-by: Ryder Lee <[email protected]>
> ---
> drivers/pci/controller/pcie-mediatek.c | 19 +++++++++++++++++--
> 1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index 861dda6..20b9088 100644
> --- a/drivers/pci/controller/pcie-mediatek.c
> +++ b/drivers/pci/controller/pcie-mediatek.c
> @@ -337,11 +337,26 @@ 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;
> + struct pci_bus *pbus;
>
> - list_for_each_entry(port, &pcie->ports, list)
> - if (port->slot == PCI_SLOT(devfn))
> + list_for_each_entry(port, &pcie->ports, list) {
> + if (!bus->number && port->slot == PCI_SLOT(devfn))
> return port;
>
> + if (bus->number) {
> + pbus = bus;
> +
> + while (pbus->number) {
> + dev = pbus->self;
> + pbus = dev->bus;
> + }
> +
> + if (port->slot == PCI_SLOT(dev->devfn))
> + return port;
> + }
> + }
/*
* Walk the bus hierarchy to get the devfn value
* of the port in the root bus.
*/
while (bus && bus->number) {
devfn = bus->self->devfn;
bus = bus->parent;
}
list_for_each_entry(port, &pcie->ports, list) {
if (port->slot == PCI_SLOT(devfn))
return port;
return NULL;
Would it work ? Maybe it is a little easier to parse.
I think this can even be made easier by using struct device_node* as
comparison (and store it in port->slot instead of the port number), I
think that the pci_dev representing the port should have the right
of_node associated with it by core code, so it is just a matter of
finding the first pci_dev parent with an of_node set and compare it
against port->slot (that should be converted to a struct device_node*).
This ought to be tested and written but I think that's doable.
Lorenzo
On Mon, Sep 10, 2018 at 05:50:21PM +0800, [email protected] wrote:
> From: Honghui Zhang <[email protected]>
>
> 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.
>
> The function of mtk_pcie_startup_port_v2's define location is
> re-arranged to avoid mtk_pcie_enable_msi's forward declaration.
>
> Signed-off-by: Honghui Zhang <[email protected]>
> Reviewed-by: Ryder Lee <[email protected]>
> ---
> drivers/pci/controller/pcie-mediatek.c | 143 +++++++++++++++++----------------
> 1 file changed, 72 insertions(+), 71 deletions(-)
Can you read:
https://marc.info/?l=linux-pci&m=150905742808166&w=2
follow it and adapt this patch and the others accordingly please ?
Thanks,
Lorenzo
> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index 20b9088..5aba43a 100644
> --- a/drivers/pci/controller/pcie-mediatek.c
> +++ b/drivers/pci/controller/pcie-mediatek.c
> @@ -398,75 +398,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_HOST;
> - 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);
> @@ -643,8 +574,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;
> @@ -711,6 +640,78 @@ 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_HOST;
> + 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);
> +
> + 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));
> + 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
>
On Fri, 2018-09-21 at 17:07 +0100, Lorenzo Pieralisi wrote:
> On Mon, Sep 10, 2018 at 05:50:20PM +0800, [email protected] wrote:
> > From: Honghui Zhang <[email protected]>
> >
> > 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 logical by using the slot's
> > devfn for match.
> >
> > Signed-off-by: Honghui Zhang <[email protected]>
> > Acked-by: Ryder Lee <[email protected]>
> > ---
> > drivers/pci/controller/pcie-mediatek.c | 19 +++++++++++++++++--
> > 1 file changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> > index 861dda6..20b9088 100644
> > --- a/drivers/pci/controller/pcie-mediatek.c
> > +++ b/drivers/pci/controller/pcie-mediatek.c
> > @@ -337,11 +337,26 @@ 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;
> > + struct pci_bus *pbus;
> >
> > - list_for_each_entry(port, &pcie->ports, list)
> > - if (port->slot == PCI_SLOT(devfn))
> > + list_for_each_entry(port, &pcie->ports, list) {
> > + if (!bus->number && port->slot == PCI_SLOT(devfn))
> > return port;
> >
> > + if (bus->number) {
> > + pbus = bus;
> > +
> > + while (pbus->number) {
> > + dev = pbus->self;
> > + pbus = dev->bus;
> > + }
> > +
> > + if (port->slot == PCI_SLOT(dev->devfn))
> > + return port;
> > + }
> > + }
>
> /*
> * Walk the bus hierarchy to get the devfn value
> * of the port in the root bus.
> */
> while (bus && bus->number) {
> devfn = bus->self->devfn;
> bus = bus->parent;
> }
>
> list_for_each_entry(port, &pcie->ports, list) {
> if (port->slot == PCI_SLOT(devfn))
> return port;
>
> return NULL;
>
> Would it work ? Maybe it is a little easier to parse.
>
Thanks, this is much better.
> I think this can even be made easier by using struct device_node* as
> comparison (and store it in port->slot instead of the port number), I
> think that the pci_dev representing the port should have the right
> of_node associated with it by core code, so it is just a matter of
> finding the first pci_dev parent with an of_node set and compare it
> against port->slot (that should be converted to a struct device_node*).
>
> This ought to be tested and written but I think that's doable.
>
Thanks, I will do my homework and figure a way out through this
solution.
Currently port->slot is used in too much place to be removed, I guess I
need to added a new parameter in the port struct to store the of_node
for compare.
I'm not sure which way is better, anyway, let me try the new solution.
thanks.
> Lorenzo
On Fri, 2018-09-21 at 17:46 +0100, Lorenzo Pieralisi wrote:
> On Mon, Sep 10, 2018 at 05:50:21PM +0800, [email protected] wrote:
> > From: Honghui Zhang <[email protected]>
> >
> > 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.
> >
> > The function of mtk_pcie_startup_port_v2's define location is
> > re-arranged to avoid mtk_pcie_enable_msi's forward declaration.
> >
> > Signed-off-by: Honghui Zhang <[email protected]>
> > Reviewed-by: Ryder Lee <[email protected]>
> > ---
> > drivers/pci/controller/pcie-mediatek.c | 143 +++++++++++++++++----------------
> > 1 file changed, 72 insertions(+), 71 deletions(-)
>
> Can you read:
>
> https://marc.info/?l=linux-pci&m=150905742808166&w=2
>
> follow it and adapt this patch and the others accordingly please ?
>
Sorry, I mixed those patches together. I will split a new patch to do
the code re-arrangement to avoid forward declaration.
Thanks.
> Thanks,
> Lorenzo
>
> > diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> > index 20b9088..5aba43a 100644
> > --- a/drivers/pci/controller/pcie-mediatek.c
> > +++ b/drivers/pci/controller/pcie-mediatek.c
> > @@ -398,75 +398,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_HOST;
> > - 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);
> > @@ -643,8 +574,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;
> > @@ -711,6 +640,78 @@ 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_HOST;
> > + 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);
> > +
> > + 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));
> > + 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
> >