From: Honghui Zhang <[email protected]>
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 fixup the PCI core defect which assign resource base on device's class
type. Logically, the resource assignment should base on PCIe configuration space
layout instead of class type. So this patch using configuration header type for
resource assignment, this patch is suggested by Bjorn.
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 v8:
- Remove the patch (PCI: mediatek: Fix class type for MT7622 as PCI_CLASS_BRIDGE_PCI)
- Add patch 2 (PCI: Using PCI configuration space header type instead of class type
to assign resource)
Change since v7:
- Add Acked-by tags from Ryder Lee.
- Add Fix tags for patch 2(Fix calss type for MT7622 as PCI_CLASS_BRIDGE_PCI)
and patch 6(Fixup enable MSI logic by enable MSI after clock enabled)
Change since v6:
- Remove the pci_unmap_iospace when remove the device since the
devm_pci_remap_iospace is an devm_ version.
- Commit message changed for patch 2(Fix class type for MT7622 as PCI_CLASS_BRIDGE_PCI).
- Capitilizing "MSI" and "PM" in the patch title.
Change since v5:
- A bit improvement of mtk_pcie_find_port suggest by Lorenzo.
MSI after clock enabled.
- Add Acked-by tags from Ryder.
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.
Honghui Zhang (9):
PCI: mediatek: Using slot's devfn for compare to fix
mtk_pcie_find_port logic
PCI: Using PCI configuration space header type instead of class type
to assign resource
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: Fixup enable MSI logic by 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 | 319 +++++++++++++++++++++------------
drivers/pci/pci.c | 3 +-
drivers/pci/probe.c | 3 -
drivers/pci/setup-bus.c | 20 +--
5 files changed, 215 insertions(+), 132 deletions(-)
--
2.6.4
From: Honghui Zhang <[email protected]>
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 <[email protected]>
Acked-by: Ryder Lee <[email protected]>
---
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 6080b29..9f12b17 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -1121,34 +1121,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;
@@ -1175,7 +1147,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
From: Honghui Zhang <[email protected]>
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 <[email protected]>
Acked-by: Ryder Lee <[email protected]>
---
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 cf7e357..7f3b8a0 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;
@@ -620,7 +622,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) {
@@ -628,8 +630,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
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 | 51 +++++++++++++++++++++++++++++++++-
2 files changed, 51 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 7f3b8a0..2a1f97c 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>
@@ -532,6 +533,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)
{
@@ -1171,6 +1193,31 @@ 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_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);
@@ -1248,6 +1295,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,
@@ -1255,4 +1303,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]>
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 <[email protected]>
Acked-by: Ryder Lee <[email protected]>
---
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 82d3d85..cf7e357 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -1168,6 +1168,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,
@@ -1200,6 +1249,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
From: Honghui Zhang <[email protected]>
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 <[email protected]>
Acked-by: Ryder Lee <[email protected]>
---
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 9f12b17..6967bb7 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -392,75 +392,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);
@@ -705,6 +636,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_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 __iomem *mtk_pcie_map_bus(struct pci_bus *bus,
unsigned int devfn, int where)
{
--
2.6.4
From: Honghui Zhang <[email protected]>
The commit 43e6409db64d ("PCI: mediatek: Add MSI support for MT2712 and
MT7622") added MSI support but enable MSI in wrong place, 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.
Fixes: 43e6409db64d ("PCI: mediatek: Add MSI support for MT2712 and MT7622")
Signed-off-by: Honghui Zhang <[email protected]>
Acked-by: Ryder Lee <[email protected]>
---
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 6967bb7..82d3d85 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -568,8 +568,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;
@@ -690,6 +688,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
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 logic by using the slot's
devfn for match if finding device connected to the subordinate bus.
Signed-off-by: Honghui Zhang <[email protected]>
Acked-by: Ryder Lee <[email protected]>
---
drivers/pci/controller/pcie-mediatek.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index 9999dae..288b8e2 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -337,6 +337,17 @@ 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;
+ devfn = dev->devfn;
+ }
list_for_each_entry(port, &pcie->ports, list)
if (port->slot == PCI_SLOT(devfn))
--
2.6.4
From: Honghui Zhang <[email protected]>
The PCI configuration space header type defines the layout of the rest
of the header (PCI r3.0 sec 6.1, PCIe r4.0 sec 7.5.1.1.9) while the
resource assignment is based on the configuration space layout instead
of its class type. Using configuration space header type instead of
class type for the resource assignment.
Suggested-by: Bjorn Helgaas <[email protected]>
Signed-off-by: Honghui Zhang <[email protected]>
---
drivers/pci/pci.c | 3 +--
drivers/pci/probe.c | 3 ---
drivers/pci/setup-bus.c | 20 ++++++++++----------
3 files changed, 11 insertions(+), 15 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 29ff961..7d379ca 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5908,8 +5908,7 @@ void pci_reassigndev_resource_alignment(struct pci_dev *dev)
* to enable the kernel to reassign new resource
* window later on.
*/
- if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE &&
- (dev->class >> 8) == PCI_CLASS_BRIDGE_PCI) {
+ if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
for (i = PCI_BRIDGE_RESOURCES; i < PCI_NUM_RESOURCES; i++) {
r = &dev->resource[i];
if (!(r->flags & IORESOURCE_MEM))
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ec78400..29a35c1 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1695,9 +1695,6 @@ int pci_setup_device(struct pci_dev *dev)
break;
case PCI_HEADER_TYPE_BRIDGE: /* bridge header */
- if (class != PCI_CLASS_BRIDGE_PCI)
- goto bad;
-
/*
* The PCI-to-PCI bridge spec requires that subtractive
* decoding (i.e. transparent) bridge must have programming
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 79b1824..69f90f4 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -182,7 +182,7 @@ static void __dev_sort_resources(struct pci_dev *dev,
u16 class = dev->class >> 8;
/* Don't touch classless devices or host bridges or ioapics. */
- if (class == PCI_CLASS_NOT_DEFINED || class == PCI_CLASS_BRIDGE_HOST)
+ if (class == PCI_CLASS_NOT_DEFINED)
return;
/* Don't touch ioapic devices already enabled by firmware */
@@ -1221,12 +1221,12 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
if (!b)
continue;
- switch (dev->class >> 8) {
- case PCI_CLASS_BRIDGE_CARDBUS:
+ switch (dev->hdr_type) {
+ case PCI_HEADER_TYPE_CARDBUS:
pci_bus_size_cardbus(b, realloc_head);
break;
- case PCI_CLASS_BRIDGE_PCI:
+ case PCI_HEADER_TYPE_BRIDGE:
default:
__pci_bus_size_bridges(b, realloc_head);
break;
@@ -1237,12 +1237,12 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
if (pci_is_root_bus(bus))
return;
- switch (bus->self->class >> 8) {
- case PCI_CLASS_BRIDGE_CARDBUS:
+ switch (bus->self->hdr_type) {
+ case PCI_HEADER_TYPE_CARDBUS:
/* don't size cardbuses yet. */
break;
- case PCI_CLASS_BRIDGE_PCI:
+ case PCI_HEADER_TYPE_BRIDGE:
pci_bridge_check_ranges(bus);
if (bus->self->is_hotplug_bridge) {
additional_io_size = pci_hotplug_io_size;
@@ -1391,13 +1391,13 @@ void __pci_bus_assign_resources(const struct pci_bus *bus,
__pci_bus_assign_resources(b, realloc_head, fail_head);
- switch (dev->class >> 8) {
- case PCI_CLASS_BRIDGE_PCI:
+ switch (dev->hdr_type) {
+ case PCI_HEADER_TYPE_BRIDGE:
if (!pci_is_enabled(dev))
pci_setup_bridge(b);
break;
- case PCI_CLASS_BRIDGE_CARDBUS:
+ case PCI_HEADER_TYPE_CARDBUS:
pci_setup_cardbus(b);
break;
--
2.6.4
From: Honghui Zhang <[email protected]>
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 <[email protected]>
Acked-by: Ryder Lee <[email protected]>
---
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 288b8e2..6080b29 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)
@@ -998,10 +996,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);
@@ -1013,10 +1009,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
On Tue, Oct 16, 2018 at 06:44:43PM +0800, [email protected] wrote:
> From: Honghui Zhang <[email protected]>
>
> The PCI configuration space header type defines the layout of the rest
> of the header (PCI r3.0 sec 6.1, PCIe r4.0 sec 7.5.1.1.9) while the
> resource assignment is based on the configuration space layout instead
> of its class type. Using configuration space header type instead of
> class type for the resource assignment.
>
> Suggested-by: Bjorn Helgaas <[email protected]>
> Signed-off-by: Honghui Zhang <[email protected]>
> ---
> drivers/pci/pci.c | 3 +--
> drivers/pci/probe.c | 3 ---
> drivers/pci/setup-bus.c | 20 ++++++++++----------
> 3 files changed, 11 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 29ff961..7d379ca 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -5908,8 +5908,7 @@ void pci_reassigndev_resource_alignment(struct pci_dev *dev)
> * to enable the kernel to reassign new resource
> * window later on.
> */
> - if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE &&
> - (dev->class >> 8) == PCI_CLASS_BRIDGE_PCI) {
> + if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
> for (i = PCI_BRIDGE_RESOURCES; i < PCI_NUM_RESOURCES; i++) {
> r = &dev->resource[i];
> if (!(r->flags & IORESOURCE_MEM))
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index ec78400..29a35c1 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1695,9 +1695,6 @@ int pci_setup_device(struct pci_dev *dev)
> break;
>
> case PCI_HEADER_TYPE_BRIDGE: /* bridge header */
> - if (class != PCI_CLASS_BRIDGE_PCI)
> - goto bad;
> -
> /*
> * The PCI-to-PCI bridge spec requires that subtractive
> * decoding (i.e. transparent) bridge must have programming
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 79b1824..69f90f4 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -182,7 +182,7 @@ static void __dev_sort_resources(struct pci_dev *dev,
> u16 class = dev->class >> 8;
>
> /* Don't touch classless devices or host bridges or ioapics. */
> - if (class == PCI_CLASS_NOT_DEFINED || class == PCI_CLASS_BRIDGE_HOST)
> + if (class == PCI_CLASS_NOT_DEFINED)
I think this check has been there since the first initial git commit,
whether that's _really_ needed or not in the current kernel it is very
hard to say.
I am not that sure it is safe to remove it, especially given that we are at
-rc8 and close to a release, it would be good if this patch could sit in
next to give it some exposure to testing before merging it upstream.
Lorenzo
> return;
>
> /* Don't touch ioapic devices already enabled by firmware */
> @@ -1221,12 +1221,12 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
> if (!b)
> continue;
>
> - switch (dev->class >> 8) {
> - case PCI_CLASS_BRIDGE_CARDBUS:
> + switch (dev->hdr_type) {
> + case PCI_HEADER_TYPE_CARDBUS:
> pci_bus_size_cardbus(b, realloc_head);
> break;
>
> - case PCI_CLASS_BRIDGE_PCI:
> + case PCI_HEADER_TYPE_BRIDGE:
> default:
> __pci_bus_size_bridges(b, realloc_head);
> break;
> @@ -1237,12 +1237,12 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
> if (pci_is_root_bus(bus))
> return;
>
> - switch (bus->self->class >> 8) {
> - case PCI_CLASS_BRIDGE_CARDBUS:
> + switch (bus->self->hdr_type) {
> + case PCI_HEADER_TYPE_CARDBUS:
> /* don't size cardbuses yet. */
> break;
>
> - case PCI_CLASS_BRIDGE_PCI:
> + case PCI_HEADER_TYPE_BRIDGE:
> pci_bridge_check_ranges(bus);
> if (bus->self->is_hotplug_bridge) {
> additional_io_size = pci_hotplug_io_size;
> @@ -1391,13 +1391,13 @@ void __pci_bus_assign_resources(const struct pci_bus *bus,
>
> __pci_bus_assign_resources(b, realloc_head, fail_head);
>
> - switch (dev->class >> 8) {
> - case PCI_CLASS_BRIDGE_PCI:
> + switch (dev->hdr_type) {
> + case PCI_HEADER_TYPE_BRIDGE:
> if (!pci_is_enabled(dev))
> pci_setup_bridge(b);
> break;
>
> - case PCI_CLASS_BRIDGE_CARDBUS:
> + case PCI_HEADER_TYPE_CARDBUS:
> pci_setup_cardbus(b);
> break;
>
> --
> 2.6.4
>
On Tue, Oct 16, 2018 at 03:53:55PM +0100, Lorenzo Pieralisi wrote:
> On Tue, Oct 16, 2018 at 06:44:43PM +0800, [email protected] wrote:
> > From: Honghui Zhang <[email protected]>
> >
> > The PCI configuration space header type defines the layout of the rest
> > of the header (PCI r3.0 sec 6.1, PCIe r4.0 sec 7.5.1.1.9) while the
> > resource assignment is based on the configuration space layout instead
> > of its class type. Using configuration space header type instead of
> > class type for the resource assignment.
> >
> > Suggested-by: Bjorn Helgaas <[email protected]>
> > Signed-off-by: Honghui Zhang <[email protected]>
> > ---
> > drivers/pci/pci.c | 3 +--
> > drivers/pci/probe.c | 3 ---
> > drivers/pci/setup-bus.c | 20 ++++++++++----------
> > 3 files changed, 11 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 29ff961..7d379ca 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -5908,8 +5908,7 @@ void pci_reassigndev_resource_alignment(struct pci_dev *dev)
> > * to enable the kernel to reassign new resource
> > * window later on.
> > */
> > - if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE &&
> > - (dev->class >> 8) == PCI_CLASS_BRIDGE_PCI) {
> > + if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
> > for (i = PCI_BRIDGE_RESOURCES; i < PCI_NUM_RESOURCES; i++) {
> > r = &dev->resource[i];
> > if (!(r->flags & IORESOURCE_MEM))
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index ec78400..29a35c1 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -1695,9 +1695,6 @@ int pci_setup_device(struct pci_dev *dev)
> > break;
> >
> > case PCI_HEADER_TYPE_BRIDGE: /* bridge header */
> > - if (class != PCI_CLASS_BRIDGE_PCI)
> > - goto bad;
> > -
> > /*
> > * The PCI-to-PCI bridge spec requires that subtractive
> > * decoding (i.e. transparent) bridge must have programming
> > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> > index 79b1824..69f90f4 100644
> > --- a/drivers/pci/setup-bus.c
> > +++ b/drivers/pci/setup-bus.c
> > @@ -182,7 +182,7 @@ static void __dev_sort_resources(struct pci_dev *dev,
> > u16 class = dev->class >> 8;
> >
> > /* Don't touch classless devices or host bridges or ioapics. */
> > - if (class == PCI_CLASS_NOT_DEFINED || class == PCI_CLASS_BRIDGE_HOST)
> > + if (class == PCI_CLASS_NOT_DEFINED)
>
> I think this check has been there since the first initial git commit,
> whether that's _really_ needed or not in the current kernel it is very
> hard to say.
>
> I am not that sure it is safe to remove it, especially given that we are at
> -rc8 and close to a release, it would be good if this patch could sit in
> next to give it some exposure to testing before merging it upstream.
Yes, you're right; I think I think this is a little too risky at this
point. I'll pull this patch out and queue it up for the next cycle
(v4.21).
For v4.20, I think you should resurrect the class code patch [1]. That
should be enough to make things work in v4.20, even without this hdr_type
patch. It will also improve the lspci output, because I think it uses the
class code to look up the generic description, e.g., in this output:
00:1c.0 PCI bridge: Intel Corporation Sunrise Point-LP PCI Express Root Port (rev f1)
I think the "PCI bridge" part is based on the class code.
Bjorn
[1] https://lore.kernel.org/linux-pci/[email protected]
> > return;
> >
> > /* Don't touch ioapic devices already enabled by firmware */
> > @@ -1221,12 +1221,12 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
> > if (!b)
> > continue;
> >
> > - switch (dev->class >> 8) {
> > - case PCI_CLASS_BRIDGE_CARDBUS:
> > + switch (dev->hdr_type) {
> > + case PCI_HEADER_TYPE_CARDBUS:
> > pci_bus_size_cardbus(b, realloc_head);
> > break;
> >
> > - case PCI_CLASS_BRIDGE_PCI:
> > + case PCI_HEADER_TYPE_BRIDGE:
> > default:
> > __pci_bus_size_bridges(b, realloc_head);
> > break;
> > @@ -1237,12 +1237,12 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
> > if (pci_is_root_bus(bus))
> > return;
> >
> > - switch (bus->self->class >> 8) {
> > - case PCI_CLASS_BRIDGE_CARDBUS:
> > + switch (bus->self->hdr_type) {
> > + case PCI_HEADER_TYPE_CARDBUS:
> > /* don't size cardbuses yet. */
> > break;
> >
> > - case PCI_CLASS_BRIDGE_PCI:
> > + case PCI_HEADER_TYPE_BRIDGE:
> > pci_bridge_check_ranges(bus);
> > if (bus->self->is_hotplug_bridge) {
> > additional_io_size = pci_hotplug_io_size;
> > @@ -1391,13 +1391,13 @@ void __pci_bus_assign_resources(const struct pci_bus *bus,
> >
> > __pci_bus_assign_resources(b, realloc_head, fail_head);
> >
> > - switch (dev->class >> 8) {
> > - case PCI_CLASS_BRIDGE_PCI:
> > + switch (dev->hdr_type) {
> > + case PCI_HEADER_TYPE_BRIDGE:
> > if (!pci_is_enabled(dev))
> > pci_setup_bridge(b);
> > break;
> >
> > - case PCI_CLASS_BRIDGE_CARDBUS:
> > + case PCI_HEADER_TYPE_CARDBUS:
> > pci_setup_cardbus(b);
> > break;
> >
> > --
> > 2.6.4
> >
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Wed, 2018-10-17 at 08:22 -0500, Bjorn Helgaas wrote:
> On Tue, Oct 16, 2018 at 03:53:55PM +0100, Lorenzo Pieralisi wrote:
> > On Tue, Oct 16, 2018 at 06:44:43PM +0800, [email protected] wrote:
> > > From: Honghui Zhang <[email protected]>
> > >
> > > The PCI configuration space header type defines the layout of the rest
> > > of the header (PCI r3.0 sec 6.1, PCIe r4.0 sec 7.5.1.1.9) while the
> > > resource assignment is based on the configuration space layout instead
> > > of its class type. Using configuration space header type instead of
> > > class type for the resource assignment.
> > >
> > > Suggested-by: Bjorn Helgaas <[email protected]>
> > > Signed-off-by: Honghui Zhang <[email protected]>
> > > ---
> > > drivers/pci/pci.c | 3 +--
> > > drivers/pci/probe.c | 3 ---
> > > drivers/pci/setup-bus.c | 20 ++++++++++----------
> > > 3 files changed, 11 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > > index 29ff961..7d379ca 100644
> > > --- a/drivers/pci/pci.c
> > > +++ b/drivers/pci/pci.c
> > > @@ -5908,8 +5908,7 @@ void pci_reassigndev_resource_alignment(struct pci_dev *dev)
> > > * to enable the kernel to reassign new resource
> > > * window later on.
> > > */
> > > - if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE &&
> > > - (dev->class >> 8) == PCI_CLASS_BRIDGE_PCI) {
> > > + if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
> > > for (i = PCI_BRIDGE_RESOURCES; i < PCI_NUM_RESOURCES; i++) {
> > > r = &dev->resource[i];
> > > if (!(r->flags & IORESOURCE_MEM))
> > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > > index ec78400..29a35c1 100644
> > > --- a/drivers/pci/probe.c
> > > +++ b/drivers/pci/probe.c
> > > @@ -1695,9 +1695,6 @@ int pci_setup_device(struct pci_dev *dev)
> > > break;
> > >
> > > case PCI_HEADER_TYPE_BRIDGE: /* bridge header */
> > > - if (class != PCI_CLASS_BRIDGE_PCI)
> > > - goto bad;
> > > -
> > > /*
> > > * The PCI-to-PCI bridge spec requires that subtractive
> > > * decoding (i.e. transparent) bridge must have programming
> > > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> > > index 79b1824..69f90f4 100644
> > > --- a/drivers/pci/setup-bus.c
> > > +++ b/drivers/pci/setup-bus.c
> > > @@ -182,7 +182,7 @@ static void __dev_sort_resources(struct pci_dev *dev,
> > > u16 class = dev->class >> 8;
> > >
> > > /* Don't touch classless devices or host bridges or ioapics. */
> > > - if (class == PCI_CLASS_NOT_DEFINED || class == PCI_CLASS_BRIDGE_HOST)
> > > + if (class == PCI_CLASS_NOT_DEFINED)
> >
> > I think this check has been there since the first initial git commit,
> > whether that's _really_ needed or not in the current kernel it is very
> > hard to say.
> >
> > I am not that sure it is safe to remove it, especially given that we are at
> > -rc8 and close to a release, it would be good if this patch could sit in
> > next to give it some exposure to testing before merging it upstream.
>
I'm not sure why the first version has take care of
PCI_CLASS_BRIDGE_HOST separately, I have no idea whether there's some
host bridge device that has their resource fixed.
I agree that this patch should be hold for the moment since I have it
tested only in Mediatek's platform.
Thanks
> Yes, you're right; I think I think this is a little too risky at this
> point. I'll pull this patch out and queue it up for the next cycle
> (v4.21).
Thanks, I will follow up this patch.
>
> For v4.20, I think you should resurrect the class code patch [1]. That
> should be enough to make things work in v4.20, even without this hdr_type
> patch. It will also improve the lspci output, because I think it uses the
> class code to look up the generic description, e.g., in this output:
>
> 00:1c.0 PCI bridge: Intel Corporation Sunrise Point-LP PCI Express Root Port (rev f1)
>
> I think the "PCI bridge" part is based on the class code.
>
> Bjorn
>
> [1] https://lore.kernel.org/linux-pci/[email protected]
>
> > > return;
> > >
> > > /* Don't touch ioapic devices already enabled by firmware */
> > > @@ -1221,12 +1221,12 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
> > > if (!b)
> > > continue;
> > >
> > > - switch (dev->class >> 8) {
> > > - case PCI_CLASS_BRIDGE_CARDBUS:
> > > + switch (dev->hdr_type) {
> > > + case PCI_HEADER_TYPE_CARDBUS:
> > > pci_bus_size_cardbus(b, realloc_head);
> > > break;
> > >
> > > - case PCI_CLASS_BRIDGE_PCI:
> > > + case PCI_HEADER_TYPE_BRIDGE:
> > > default:
> > > __pci_bus_size_bridges(b, realloc_head);
> > > break;
> > > @@ -1237,12 +1237,12 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
> > > if (pci_is_root_bus(bus))
> > > return;
> > >
> > > - switch (bus->self->class >> 8) {
> > > - case PCI_CLASS_BRIDGE_CARDBUS:
> > > + switch (bus->self->hdr_type) {
> > > + case PCI_HEADER_TYPE_CARDBUS:
> > > /* don't size cardbuses yet. */
> > > break;
> > >
> > > - case PCI_CLASS_BRIDGE_PCI:
> > > + case PCI_HEADER_TYPE_BRIDGE:
> > > pci_bridge_check_ranges(bus);
> > > if (bus->self->is_hotplug_bridge) {
> > > additional_io_size = pci_hotplug_io_size;
> > > @@ -1391,13 +1391,13 @@ void __pci_bus_assign_resources(const struct pci_bus *bus,
> > >
> > > __pci_bus_assign_resources(b, realloc_head, fail_head);
> > >
> > > - switch (dev->class >> 8) {
> > > - case PCI_CLASS_BRIDGE_PCI:
> > > + switch (dev->hdr_type) {
> > > + case PCI_HEADER_TYPE_BRIDGE:
> > > if (!pci_is_enabled(dev))
> > > pci_setup_bridge(b);
> > > break;
> > >
> > > - case PCI_CLASS_BRIDGE_CARDBUS:
> > > + case PCI_HEADER_TYPE_CARDBUS:
> > > pci_setup_cardbus(b);
> > > break;
> > >
> > > --
> > > 2.6.4
> > >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tue, Oct 16, 2018 at 06:44:43PM +0800, [email protected] wrote:
> From: Honghui Zhang <[email protected]>
>
> The PCI configuration space header type defines the layout of the rest
> of the header (PCI r3.0 sec 6.1, PCIe r4.0 sec 7.5.1.1.9) while the
> resource assignment is based on the configuration space layout instead
> of its class type. Using configuration space header type instead of
> class type for the resource assignment.
>
> Suggested-by: Bjorn Helgaas <[email protected]>
> Signed-off-by: Honghui Zhang <[email protected]>
I applied the patch below to pci/enumeration for v5.1.
I dropped the hunk that removed the PCI_CLASS_BRIDGE_HOST check per
Lorenzo's concern. Let me know if you have any other concerns.
commit b2fb5cc57469
Author: Honghui Zhang <[email protected]>
Date: Tue Oct 16 18:44:43 2018 +0800
PCI: Rely on config space header type, not class code
The PCI configuration space header type tells us whether the device is a
bridge, a CardBus bridge, or a normal device, and defines the layout of the
rest of the header (PCI r3.0 sec 6.1, PCIe r4.0 sec 7.5.1.1.9).
When we rely on the header format, e.g., when we're dealing with bridge
windows, we should check the header type, not the class code. The class
code is loosely related to the header type, but is often incorrect and the
spec doesn't actually require it to be related to the header format.
Suggested-by: Bjorn Helgaas <[email protected]>
Signed-off-by: Honghui Zhang <[email protected]>
[bhelgaas: changelog, keep the PCI_CLASS_BRIDGE_HOST check]
Signed-off-by: Bjorn Helgaas <[email protected]>
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index c9d8e3c837de..e9d938e14ba8 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -6000,8 +6000,7 @@ void pci_reassigndev_resource_alignment(struct pci_dev *dev)
* to enable the kernel to reassign new resource
* window later on.
*/
- if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE &&
- (dev->class >> 8) == PCI_CLASS_BRIDGE_PCI) {
+ if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
for (i = PCI_BRIDGE_RESOURCES; i < PCI_NUM_RESOURCES; i++) {
r = &dev->resource[i];
if (!(r->flags & IORESOURCE_MEM))
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 8e2e4154cdd9..128459a0ffba 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1779,9 +1779,6 @@ int pci_setup_device(struct pci_dev *dev)
break;
case PCI_HEADER_TYPE_BRIDGE: /* bridge header */
- if (class != PCI_CLASS_BRIDGE_PCI)
- goto bad;
-
/*
* The PCI-to-PCI bridge spec requires that subtractive
* decoding (i.e. transparent) bridge must have programming
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 1941bb0a6c13..ec44a0f3a7ac 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1186,12 +1186,12 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
if (!b)
continue;
- switch (dev->class >> 8) {
- case PCI_CLASS_BRIDGE_CARDBUS:
+ switch (dev->hdr_type) {
+ case PCI_HEADER_TYPE_CARDBUS:
pci_bus_size_cardbus(b, realloc_head);
break;
- case PCI_CLASS_BRIDGE_PCI:
+ case PCI_HEADER_TYPE_BRIDGE:
default:
__pci_bus_size_bridges(b, realloc_head);
break;
@@ -1202,12 +1202,12 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
if (pci_is_root_bus(bus))
return;
- switch (bus->self->class >> 8) {
- case PCI_CLASS_BRIDGE_CARDBUS:
+ switch (bus->self->hdr_type) {
+ case PCI_HEADER_TYPE_CARDBUS:
/* don't size cardbuses yet. */
break;
- case PCI_CLASS_BRIDGE_PCI:
+ case PCI_HEADER_TYPE_BRIDGE:
pci_bridge_check_ranges(bus);
if (bus->self->is_hotplug_bridge) {
additional_io_size = pci_hotplug_io_size;
@@ -1356,13 +1356,13 @@ void __pci_bus_assign_resources(const struct pci_bus *bus,
__pci_bus_assign_resources(b, realloc_head, fail_head);
- switch (dev->class >> 8) {
- case PCI_CLASS_BRIDGE_PCI:
+ switch (dev->hdr_type) {
+ case PCI_HEADER_TYPE_BRIDGE:
if (!pci_is_enabled(dev))
pci_setup_bridge(b);
break;
- case PCI_CLASS_BRIDGE_CARDBUS:
+ case PCI_HEADER_TYPE_CARDBUS:
pci_setup_cardbus(b);
break;