On Thu, 16 Mar 2023 10:47:11 +0000, Lorenzo Pieralisi lpieralisi@kernel.org wrote:
On Thu, Mar 16, 2023 at 09:32:35AM +0000, Marc Zyngier wrote:
On Thu, 09 Mar 2023 16:39:35 +0000, Bjorn Helgaas helgaas@kernel.org wrote:
[+cc Daire, Conor for apple/microchip use of ECAM .init() method]
On Thu, Mar 09, 2023 at 02:36:24PM +0100, Janne Grunau wrote:
Fixes following warning inside of_irq_parse_raw() called from the common PCI device probe path.
/soc/pcie@690000000/pci@1,0 interrupt-map failed, using interrupt-controller WARNING: CPU: 4 PID: 252 at drivers/of/irq.c:279 of_irq_parse_raw+0x5fc/0x724
Based on this commit log, I assume this patch only fixes the warning, and the system *works* just fine either way. If that's the case, it's debatable whether it meets the stable kernel criteria, although the documented criteria are much stricter than what happens in practice.
... Call trace: of_irq_parse_raw+0x5fc/0x724 of_irq_parse_and_map_pci+0x128/0x1d8 pci_assign_irq+0xc8/0x140 pci_device_probe+0x70/0x188 really_probe+0x178/0x418 __driver_probe_device+0x120/0x188 driver_probe_device+0x48/0x22c __device_attach_driver+0x134/0x1d8 bus_for_each_drv+0x8c/0xd8 __device_attach+0xdc/0x1d0 device_attach+0x20/0x2c pci_bus_add_device+0x5c/0xc0 pci_bus_add_devices+0x58/0x88 pci_host_probe+0x124/0x178 pci_host_common_probe+0x124/0x198 [pci_host_common] apple_pcie_probe+0x108/0x16c [pcie_apple] platform_probe+0xb4/0xdc
This became apparent after disabling unused PCIe ports in the Apple silicon device trees instead of deleting them.
Use for_each_available_child_of_node instead of for_each_child_of_node which takes the "status" property into account.
Link: https://lore.kernel.org/asahi/20230214-apple_dts_pcie_disable_unused-v1-0-5e... Link: https://lore.kernel.org/asahi/1ea2107a-bb86-8c22-0bbc-82c453ab08ce@linaro.or... Fixes: 1e33888fbe44 ("PCI: apple: Add initial hardware bring-up") Cc: stable@vger.kernel.org Reviewed-by: Marc Zyngier maz@kernel.org Signed-off-by: Janne Grunau j@jannau.net
Changes in v2:
- rewritten commit message with more details and corrections
- collected Marc's "Reviewed-by:"
- Link to v1: https://lore.kernel.org/r/20230307-apple_pcie_disabled_ports-v1-1-b32ef91faf...
drivers/pci/controller/pcie-apple.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c index 66f37e403a09..f8670a032f7a 100644 --- a/drivers/pci/controller/pcie-apple.c +++ b/drivers/pci/controller/pcie-apple.c @@ -783,7 +783,7 @@ static int apple_pcie_init(struct pci_config_window *cfg) cfg->priv = pcie; INIT_LIST_HEAD(&pcie->ports);
- for_each_child_of_node(dev->of_node, of_port) {
- for_each_available_child_of_node(dev->of_node, of_port) { ret = apple_pcie_setup_port(pcie, of_port); if (ret) { dev_err(pcie->dev, "Port %pOF setup fail: %d\n", of_port, ret);
Is this change still needed after 6fffbc7ae137 ("PCI: Honor firmware's device disabled status")? This is a generic problem, and it would be a lot nicer if we had a generic solution. But I assume it *is* still needed because Rob gave his Reviewed-by.
I'm not sure this is addressing the same issue. The way I read it, the patch you mention here allows a PCI device to be disabled in firmware, even if it could otherwise be probed.
What this patch does is to prevent root ports that exist in the HW but that have been disabled from being probed. Same concept, only at a different level.
A root port is a PCI device though and that's what's causing the warning AFAIK (? it is triggered on the root port PCI device pci_assign_irq() call),
As usual, there are two sides to things. The root ports are also part of a platform device, and this what this patch uses.
I am not sure the root port DT node is associated with the root port PCI device correctly, which might explain why, even after 6fffbc7ae137, the PCI enumeration code is adding the root port PCI device to the PCI tree.
I didn't say that commit did not suppress the warning. I haven't tested it the first place because I really need all the PCIe ports I can get on the machines I have.
It just feels to me that they are tracking different things.
Is the dts available anywhere ? How are root ports described in it ?
arch/arm64/boot/dts/apple/t8103.dtsi Documentation/devicetree/bindings/pci/apple,pcie.yaml
M.